Jan 5, 2021, 10:43 by nicolas.gaullier@cji.paris: >>>> De : Nicolas Gaullier <nicolas.gaullier@cji.paris> Envoyé : mardi 15 >>>> décembre 2020 18:13 À : ffmpeg-devel@ffmpeg.org Cc : Nicolas Gaullier >>>> <nicolas.gaullier@cji.paris> Objet : [PATCH] avcodec/dolby_e: set >>>> constant frame_size >>>> >>>> Fixes pts generation. >>>> >>>> Setting frame_size in dolby_e_init() or get_audio_frame_duration() can >>>> result in a bad duration value for the first packet if dolby_e is muxed in >>>> a container having a different sample_rate (ex: >>>> container @48KHz, DolbyE @44.8KHz). >>>> Maybe adding a parser to dolby_e would fix the issue and makes it possible >>>> to set frame_size at decoder init which seems the best place. >>>> > >I am not sure I understand this. It is suprising that you say that > >frame_size cannot be set in dolby_e_init(), why does it matter? It can only > >be FRAME_SAMPLES, no other values can happen. In that sense it is similar to > >sample_fmt, which I also don't see why it is set in every decode call, and > >not only once, in init. > > Yes, this is not easy to see because the current code does not make this > problem show up, but my initial target is a patch serie to support dolby_e in > the wav container, and it makes it very clear. > Please look at : > https://patchwork.ffmpeg.org/project/ffmpeg/patch/20201019081929.1926-9-nicolas.gaullier@cji.paris/ > The issue is that the WAV container has typically a sample rate of 48000Hz > which contains the s377m submux that embedd a 44800Hz stream ("pal"-DolbyE > here) : all this is very typical, standard, DolbyE content, but there is a > trick in compute_pkt_fields (if I remember correctly my testing) with early > duration setting based on 48000Hz which results in a wrong value for the > first frame. > Maybe having a DolbyE parser would fix this (the 44800Hz would araise > sooner), but currently the pts are broken. > > Here is the diff if I set frame_size at dolby_e_init: > --- ./tests/ref/fate/s337m-wav 2020-12-15 18:02:28.166747900 +0100 > +++ tests/data/fate/s337m-wav 2021-01-05 10:27:01.193976500 +0100 > @@ -4,8 +4,8 @@ > #sample_rate 0: 44800 > #channel_layout 0: 63f > #channel_layout_name 0: 7.1 > -0, 0, 0, 1920, 11496, 0x05a9c147 > -0, 1920, 1920, 1920, 11496, 0x1d44d2b4 > -0, 3840, 3840, 1920, 11496, 0x4e078953 > -0, 5760, 5760, 1920, 11496, 0x1c73b1a1 > -0, 7680, 7680, 1920, 11262, 0xfa179fc8 > +0, 0, 0, 1792, 11496, 0x05a9c147 > +0, 1792, 1792, 1920, 11496, 0x1d44d2b4 > +0, 3712, 3712, 1920, 11496, 0x4e078953 > +0, 5632, 5632, 1920, 11496, 0x1c73b1a1 > +0, 7552, 7552, 1920, 11262, 0xfa179fc8 > >>>> --- >>>> libavcodec/dolby_e.c | 1 + >>>> 1 file changed, 1 insertion(+) >>>> >>>> diff --git a/libavcodec/dolby_e.c b/libavcodec/dolby_e.c index >>>> 429612ec08..b0e6d6aee3 100644 >>>> --- a/libavcodec/dolby_e.c >>>> +++ b/libavcodec/dolby_e.c >>>> @@ -577,6 +577,7 @@ static int filter_frame(DBEContext *s, AVFrame *frame) >>>> reorder = ch_reorder_n; >>>> >>>> frame->nb_samples = FRAME_SAMPLES; >>>> + s->avctx->frame_size = FRAME_SAMPLES; >>>> > >If you still believe that setting this is required in every decode call, > >then I'd say it would be cleaner to set this at dolby_e_decode_frame where > >other avctx parameters are also set. > >> >> > >Thanks, > >Marton > > I agree with you that sample_fmt and frame_size are both "const" and should > probably be set at the same place wherever it is, and preferably at > dolby_e_init. > I cannot set frame_size in dolby_e_init because of the trick and sample_fmt > is already set at dolby_e_decode_frame (for an unknown reason), maybe I > should set frame_size in dolby_e_decode_frame too. > > I have just tested setting frame_size at dolby_e_decode_frame, and I confirm > : yes, it works. > This is not ideal but in the very short term, I really cannot see any other > option : will you approve the patch if I set frame_size at > dolby_e_decode_frame instead of filter_frame ? Should I amend my commit msg? >
I still don't get it. It really does seem like a hack or a workaround to set the frame size on every single frame. In general, frame_size for decoders is read only. If something's touching it apart from the decoder, then its an API misuse. _______________________________________________ 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".
