[FFmpeg-devel] [PATCH] avformat/oggparsevorbis: Update end_trimming for the last packet

[Lynne]

6 Jul 2021, 08:02 by sunguangyucn at gmail.com:

> On Mon, Jun 21, 2021 at 9:06 AM Guangyu Sun <sunguangyucn at gmail.com> wrote:
>
>>
>> From: Guangyu Sun <sunguangyucn at gmail.com>
>>
>> Without end_trimming, the last packet will contain unexpected samples used
>> for padding.
>>
>> This commit partially fixes #6367 when the audio length is long enough.
>>
>> dd if=/dev/zero of=./silence.raw count=20 bs=500
>> oggenc --raw silence.raw --output=silence.ogg
>> oggdec --raw --output silence.oggdec.raw silence.ogg
>> ffmpeg -codec:a libvorbis -i silence.ogg -f s16le -codec:a pcm_s16le silence.libvorbis.ffmpeg.raw
>> ffmpeg -i silence.ogg -f s16le -codec:a pcm_s16le silence.native.ffmpeg.raw
>> ls -l *.raw
>>
>> The original test case in #6367 is still not fixed due to a remaining issue.
>>
>> The remaining issue is that ogg_stream->private is not kept during
>> ogg_save()/ogg_restore(). Field final_duration in the private data is
>> important to calculate end_trimming.
>>
>> Some common operations such as avformat_open_input() and
>> avformat_find_stream_info() before reading packet will trigger ogg_save()
>> and ogg_restore().
>>
>> Luckily, final_duration will not get updated until the last ogg page. The
>> save/restore mentioned above will not change final_duration most of the
>> time. But if the audio length is short, those reads may be performed on
>> the last ogg page, causing trouble keeping the correct value of
>> final_duration. We probably need a more complicated patch to address this
>> issue.
>>
>> Signed-off-by: Guangyu Sun <sunguangyucn at gmail.com>
>> ---
>>  libavformat/oggparsevorbis.c | 6 +++++-
>>  1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/libavformat/oggparsevorbis.c b/libavformat/oggparsevorbis.c
>> index 0e8c25c030..c48658ceda 100644
>> --- a/libavformat/oggparsevorbis.c
>> +++ b/libavformat/oggparsevorbis.c
>> @@ -492,8 +492,12 @@ static int vorbis_packet(AVFormatContext *s, int idx)
>>  priv->final_pts      = os->lastpts;
>>  priv->final_duration = 0;
>>  }
>> -        if (os->segp == os->nsegs)
>> +        if (os->segp == os->nsegs) {
>> +            int64_t skip = priv->final_pts + priv->final_duration + os->pduration - os->granule;
>> +            if (skip > 0)
>> +                os->end_trimming = skip;
>>  os->pduration = os->granule - priv->final_pts - priv->final_duration;
>> +        }
>>  priv->final_duration += os->pduration;
>>  }
>>
>> --
>> 2.30.1
>>
>
> After fixing AV_PKT_DATA_SKIP_SAMPLES for reading vorbis packets from
> ogg, the actual decoded samples become fewer. As a result, three fate
> tests are failing:
>
> fate-vorbis-encode:
> This exposes another bug in the vorbis encoder that initial_padding is
> not set. I will fix that in a separate patch.
>
> fate-webm-dash-chapters:
> The original vorbis_chapter_extension_demo.ogg is transmuxed to
> dash-webm. The ref file webm-dash-chapters needs to be updated.
>
> fate-vorbis-20:
> The samples in 6.ogg are not frame aligned. 6.pcm file was generated
> by ffmpeg before the fix. After the fix, the decoded pcm file does not
> match anymore. The ref file 6.pcm needs to be updated.
>
> My question is that to fix fate-vorbis-20, a new file 6_v2.pcm needs
> to be uploaded to the fate suite server. How should I do that? The doc
> says it should be sent to samples-request but I am not sure if it is
> an email or something else.
>

Just put a link to it here and someone will upload it. Then a day will have
to pass to let all fate clients pull it before pushing the patch.
Patch looks good to me as-is, since it's just a copy of what Opus does.