On 5/26/2016 11:05 PM, Michael Niedermayer wrote: > On Wed, May 25, 2016 at 02:18:38PM -0400, Ronald S. Bultje wrote: >> Hi, >> >> On Wed, May 25, 2016 at 1:24 PM, James Almer <jamr...@gmail.com> wrote: >> >>> On 5/25/2016 1:56 PM, Jon Toohill wrote: >>>> --- >>>> libavformat/mp3dec.c | 2 ++ >>>> 1 file changed, 2 insertions(+) >>>> >>>> diff --git a/libavformat/mp3dec.c b/libavformat/mp3dec.c >>>> index 3725d67..192f5ef 100644 >>>> --- a/libavformat/mp3dec.c >>>> +++ b/libavformat/mp3dec.c >>>> @@ -234,6 +234,8 @@ static void mp3_parse_info_tag(AVFormatContext *s, >>> AVStream *st, >>>> >>>> mp3->start_pad = v>>12; >>>> mp3-> end_pad = v&4095; >>>> + st->codecpar->initial_padding = mp3->start_pad; >>>> + st->codecpar->trailing_padding = mp3->end_pad; >>> >>> Every other format so far is using the AV_PKT_DATA_SKIP_SAMPLES side data >>> to >>> discard samples from the last packet/frame. See matroska and ogg demuxers, >>> currently used for Opus only. >>> >>> The trailing_padding AVCodecParameters element was added after the above >>> was >>> designed. To be honest i can't say if we should replace one with the other >>> or find a way to keep both, and seeing how AVCodecParameters hasn't made it >>> to a release yet, we're still on time to choose. >> >> >> I agree having 1 is better than having 2. I can't technically comment on >> which one is better/easier/*. > > mp3 supports AV_PKT_DATA_SKIP_SAMPLES since FFmpeg 2.5 > ogg opus sets codecpar->initial_padding like this patch would > so does dtshddec and matroskadec > > AV_PKT_DATA_SKIP_SAMPLES is not a substitute for setting > trailing_padding because with AV_PKT_DATA_SKIP_SAMPLES the > trailing_padding only becomes available at the end of the stream > > also theres nothing wrong with AV_PKT_DATA_SKIP_SAMPLES, its local > information for the current packet and there may indeed be cases > like with concatenated streams where there are packets in the middle > with discarding. So the 2 AVCodecParameters fields are not a substitute > for AV_PKT_DATA_SKIP_SAMPLES >
Fine with me, then. I assumed both both were pretty much made for the same purpose and to cover the same use cases. > also for encoding AV_PKT_DATA_SKIP_SAMPLES does not work with some > formats the trailing_padding is needed to be known when writing the > header The one format i know should be using AV_PKT_DATA_SKIP_SAMPLES the same way as ogg and matroska is mpegts. Probably also mp4. At least for Opus the mpegts muxer reads the value from the last packet the encoder or demuxer sends its way and writes it to the output file, but the mpegts demuxer is apparently none the wiser because the opus parser just discards this information. > > the patch does look like a reasonable step to me but i might be > missing something _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
