>>> 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? Thanks to you Nicolas _______________________________________________ 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".
