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

Guangyu Sun sunguangyucn at gmail.com
Tue Jul 6 19:21:08 EEST 2021


On Tue, Jul 6, 2021 at 1:15 AM Lynne <dev at lynne.ee> wrote:
>
> 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.

Great! Here is the link:
https://tinyurl.com/hkmwdk78

I would like it to be uploaded to:
fate-suite/vorbis/6_v2.pcm

Thanks in advance!

Best,
Guangyu

> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".


More information about the ffmpeg-devel mailing list