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

[Guangyu Sun]

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.