Refactor AVDictionary clean up (#2507)

Summary:
Small clean up in ffmpeg binding code.

1. Make `get_option_dict` and `clean_up_dict` public utility
2. Merge the exception into `clean_up_dict`
3. Get rid of custom string join function and use `c10::Join`.

Pull Request resolved: https://github.com/pytorch/audio/pull/2507

Reviewed By: hwangjeff

Differential Revision: D37466022

Pulled By: mthrok

fbshipit-source-id: 44b769ac6ff1ab20e6d6ae086cd1447deacb5969

test/torchaudio_unittest/io/stream_reader_test.py

@@ -83,7 +83,7 @@ class StreamReaderInterfaceTest(_MediaSourceMixin, TempDirMixin, TorchaudioTestC
         options.update({k: k for k in invalid_keys})
         with self.assertRaises(RuntimeError) as ctx:
             StreamReader(self.get_src(), option=options)
-        assert all(f'"{k}"' in str(ctx.exception) for k in invalid_keys)
+        assert all(k in str(ctx.exception) for k in invalid_keys)
 
     def test_src_info(self):
         """`get_src_stream_info` properly fetches information"""

torchaudio/csrc/ffmpeg/ffmpeg.cpp

@@ -9,13 +9,8 @@ namespace torchaudio {
 namespace ffmpeg {
 
 ////////////////////////////////////////////////////////////////////////////////
-// AVFormatContext
+// AVDictionary
 ////////////////////////////////////////////////////////////////////////////////
-void AVFormatContextDeleter::operator()(AVFormatContext* p) {
-  avformat_close_input(&p);
-};
-
-namespace {
 
 AVDictionary* get_option_dict(const OptionDict& option) {
   AVDictionary* opt = nullptr;
@@ -25,33 +20,23 @@ AVDictionary* get_option_dict(const OptionDict& option) {
   return opt;
 }
 
-std::vector<std::string> clean_up_dict(AVDictionary* p) {
-  std::vector<std::string> ret;
-
+void clean_up_dict(AVDictionary* p) {
+  std::vector<std::string> unused_keys;
   // Check and copy unused keys, clean up the original dictionary
   AVDictionaryEntry* t = nullptr;
-  do {
-    t = av_dict_get(p, "", t, AV_DICT_IGNORE_SUFFIX);
-    if (t) {
-      ret.emplace_back(t->key);
-    }
-  } while (t);
+  while ((t = av_dict_get(p, "", t, AV_DICT_IGNORE_SUFFIX))) {
+    unused_keys.emplace_back(t->key);
+  }
   av_dict_free(&p);
-  return ret;
-}
 
-std::string join(std::vector<std::string> vars) {
-  std::stringstream ks;
-  for (size_t i = 0; i < vars.size(); ++i) {
-    if (i == 0) {
-      ks << "\"" << vars[i] << "\"";
-    } else {
-      ks << ", \"" << vars[i] << "\"";
-    }
+  if (!unused_keys.empty()) {
+    throw std::runtime_error(
+        "Unexpected options: " + c10::Join(", ", unused_keys));
   }
-  return ks.str();
 }
 
+namespace {
+
 // https://github.com/FFmpeg/FFmpeg/blob/4e6debe1df7d53f3f59b37449b82265d5c08a172/doc/APIchanges#L252-L260
 // Starting from libavformat 59 (ffmpeg 5),
 // AVInputFormat is const and related functions expect constant.
@@ -63,6 +48,13 @@ std::string join(std::vector<std::string> vars) {
 
 } // namespace
 
+////////////////////////////////////////////////////////////////////////////////
+// AVFormatContext
+////////////////////////////////////////////////////////////////////////////////
+void AVFormatContextDeleter::operator()(AVFormatContext* p) {
+  avformat_close_input(&p);
+};
+
 AVFormatContextPtr get_input_format_context(
     const std::string& src,
     const c10::optional<std::string>& device,
@@ -93,12 +85,7 @@ AVFormatContextPtr get_input_format_context(
 
   AVDictionary* opt = get_option_dict(option);
   int ret = avformat_open_input(&pFormat, src.c_str(), pInput, &opt);
-
-  auto unused_keys = clean_up_dict(opt);
-
-  if (unused_keys.size()) {
-    throw std::runtime_error("Unexpected options: " + join(unused_keys));
-  }
+  clean_up_dict(opt);
 
   if (ret < 0)
     throw std::runtime_error(
@@ -293,15 +280,12 @@ void init_codec_context(
 
   AVDictionary* opts = get_option_dict(decoder_option);
   ret = avcodec_open2(pCodecContext, pCodec, &opts);
+  clean_up_dict(opts);
+
   if (ret < 0) {
     throw std::runtime_error(
         "Failed to initialize CodecContext: " + av_err2string(ret));
   }
-  auto unused_keys = clean_up_dict(opts);
-  if (unused_keys.size()) {
-    throw std::runtime_error(
-        "Unexpected decoder options: " + join(unused_keys));
-  }
 
   if (pParams->codec_type == AVMEDIA_TYPE_AUDIO && !pParams->channel_layout)
     pParams->channel_layout =

torchaudio/csrc/ffmpeg/ffmpeg.h

@@ -60,6 +60,18 @@ class Wrapper {
   }
 };
 
+////////////////////////////////////////////////////////////////////////////////
+// AVDictionary
+////////////////////////////////////////////////////////////////////////////////
+// Since AVDictionaries are relocated by FFmpeg APIs it does not suit to
+// IIRC-semantic. Instead we provide helper functions.
+
+// Convert standard dict to FFmpeg native type
+AVDictionary* get_option_dict(const OptionDict& option);
+
+// Clean up the dict after use. If there is an unsed key, throw runtime error
+void clean_up_dict(AVDictionary* p);
+
 ////////////////////////////////////////////////////////////////////////////////
 // AVFormatContext
 ////////////////////////////////////////////////////////////////////////////////