[FFmpeg-devel] [PATCH 1/3] avcodec/avpacket: add av_packet_resize()

[Michael Niedermayer]

On Sun, Mar 14, 2021 at 04:51:44PM +0100, Anton Khirnov wrote:
> Quoting James Almer (2021-03-14 16:35:48)
> > On 3/14/2021 7:34 AM, Anton Khirnov wrote:
> > > Quoting James Almer (2021-03-12 18:24:47)
> > >>
> > >> The padding exists AFAIK because something, like an optimized bitstream
> > >> reader that tries to process several bytes at the same time, may end up
> > >> reading or writing pass the reported end of the buffer.
> > >> That means that if reading and it's not all zeroes, it could in theory
> > >> have unpredictable results. Hence why everything always zeroes the
> > >> padding, even av_shrink_packet().
> > > 
> > > On that topic, I've been wondering about eliminating this requirement.
> > > Does anyone know which code it is precisely that depends on the padding
> > > being zeroed? Is this optimization really worth it?
> > > It seems rather silly to jump through all these hoops for an
> > > unmeasurable benefit in one decoder.
> > 
> > Some subtitle demuxers didn't look at pkt->size and depended on the 
> > padding to work as a 0 string terminator, but Marton fixed that in a 
> > patchset sent yesterday.
> > 
> > Beyond that, i think the v210 decoder and encoder read and write past 
> > the end of the buffer if you use the simd functions.
> > 
> > > 
> > > It would also give us zero-copy packet splitting.
> > > 
> > > [...]
> > > 
> > > I would suggest keeping av_shrink_packet() with a big scary warning that
> > > it does not check for ownership and it is the caller's responsibility to
> > > ensure that writing to the packet is safe.
> > 
> > If we can remove the zero-padding requirement, or the padding 
> > requirement altogether, then that would no longer be necessary.
> 
> I don't think we can remove the padding requirement completely, as the
> bitstream reader reads 4- or 8-byte chunks at once. I imagine changing
> that into single-byte reads would be very slow.
> But getting rid of the zeroing requirement should be more feasible.

mpeg/itu codecs historically always had the all 0 VLC of all relevant tables
invalid. That was done to ensure a long string of zeros cannot occur. and
thus a startcode as used in MPEG-PS/TS or in slices (nowadays NALs and such)
cannot occur in the middle of a valid frame.
That way simply checking for the vlc to be valid would also always check
for a startcode or the end if there where 3-4 zero bytes at the end.
This was probably used in every decoder where it could be used as
it avoided an extra check per vlc reading loop.
I have the suspicion it would be faster to check the padding for being 0
and malloc/memcpy if not in many cases than to add extra checks in every
affected loop. More so as this only affcts cases where there is no
"NAL/slice/packet" with a startcode afterwards

Thanks

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Many that live deserve death. And some that die deserve life. Can you give
it to them? Then do not be too eager to deal out death in judgement. For
even the very wise cannot see all ends. -- Gandalf
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 195 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20210314/4b8e2bed/attachment.sig>