On 3/21/2022 4:51 AM, Anton Khirnov wrote:
Quoting Hendrik Leppkes (2022-03-21 08:37:43)
On Mon, Mar 21, 2022 at 12:38 AM Andreas Rheinhardt
<andreas.rheinha...@outlook.com> wrote:

James Almer:
The function is not meant to clear codec parameters, and the lavf demux code
relies on this behavior.
Regression since 327efa66331ebdc0087c6b656059a8df2f404019.

Signed-off-by: James Almer <jamr...@gmail.com>
---
  libavcodec/avcodec.c | 6 ++++++
  1 file changed, 6 insertions(+)

diff --git a/libavcodec/avcodec.c b/libavcodec/avcodec.c
index 38bdaad4fa..253c9f56cc 100644
--- a/libavcodec/avcodec.c
+++ b/libavcodec/avcodec.c
@@ -469,6 +469,7 @@ void avsubtitle_free(AVSubtitle *sub)

  av_cold int avcodec_close(AVCodecContext *avctx)
  {
+    AVChannelLayout ch_layout;
      int i;

      if (!avctx)
@@ -524,7 +525,12 @@ av_cold int avcodec_close(AVCodecContext *avctx)

      if (avctx->priv_data && avctx->codec && avctx->codec->priv_class)
          av_opt_free(avctx->priv_data);
+    /* av_opt_free() will uninitialize avctx->ch_layout, but we want to keep 
it.
+       It will be uninitialized in avcodec_free_context() */
+    ch_layout = avctx->ch_layout;
+    memset(&avctx->ch_layout, 0, sizeof(avctx->ch_layout));
      av_opt_free(avctx);
+    avctx->ch_layout = ch_layout;
      av_freep(&avctx->priv_data);
      if (av_codec_is_encoder(avctx->codec)) {
          av_freep(&avctx->extradata);

avcodec_close() has always* called av_opt_free() and therefore
uninitialized allocated options (the documentation of avcodec_close()
states that it "frees all the data associated with it"); the new channel
layout API is (potentially) based on allocations, so ch_layout belongs
to this group of elements.
If the demux code wants to preserve ch_layout, then the demux code
should do it itself; it should not life in avcodec_close() (where it
would be a hack).


But is it expected that this is going to happen? It doesn't reset any
other codec properties, and if I set ch_layout without using avoptions
(you know, like 99% of all callers, if you use an codec context in
code then avoptions are rather clumsy), would I expect it to be reset?
ch_layout just happens to get reset because it potentially has
allocations, not because every codec property gets reset, its a
technical detail not a logical conclusion. I think there is such a
thing as being too strict about weird semantics here.

People should not use avcodec_close() anyway.

What's your opinion on the "don't free" AVOption flag Hendrik suggested?
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Reply via email to