On 3/11/2021 7:40 PM, Marton Balint wrote:
On Thu, 11 Mar 2021, James Almer wrote:
On 3/11/2021 1:11 PM, Marton Balint wrote:
On Thu, 11 Mar 2021, James Almer wrote:
This function acts as a replacement for both av_grow_packet() and
av_shrink_packet(), the latter which is now deprecated and will be
removed as
it does not correctly handle non-writable packets.
I don't think this is a good idea, av_shrink_packet cannot fail,
av_grow_packet can. By using the same function you are losing the
information if the end result should be checked or not.
I'm not sure i follow. av_shrink_packet() is not being changed at all,
just deprecated, scheduled for removal, and its use discouraged.
But why are you complicating code with mandatory return value checking?
Because unlike av_shrink_packet(), av_packet_resize() is more thorough
when handling AVPackets and allows new usage scenarios that were not
allowed with the former, thus it can fail.
Because as soon as a function returns an error, you have to check it,
and forward it upwards.
Is error checking that much of a problem? I don't understand why it
bothers people so much.
Maybe i should have split this in two, one to add av_packet_resize()
and one to deprecate av_shrink_packet(), to avoid confusion.
In any case, the fact av_shrink_packet() cannot fail is the reason I'm
getting rid of it. It's zeroing the padding without bothering to check
if the packet is writable at all.
Add an assert to it then.
We only assert on internal bugs, not invalid arguments passed by callers
to a public function. The asserts would need to be added above each
av_shrink_packet() call. And that's only for our internal uses of the
function. It does nothing to the issue of it being public API that can't
properly handle AVPackets in their current form.
> Shrinking a non-writable packet seems bad API usage anyways.
I get a packet from a demuxer. It contains two independent portions
(NALUs, OBUs, etc) i want to separate in order to feed them to something
like a multi threaded decoder. And so I create a new reference to it,
resulting in two packets pointing to the same data. I shrink one to only
contain the first portion, and i add the required byte offset to the
data pointer and subtract it to the size field on the second packet so
it contains only the second portion.
The result if i use av_packet_resize() will be two valid, properly
padded packets containing their respective expected data, but if i use
av_shrink_packet(), the result will be the packet with the second
portion featuring padding bytes worth of data zeroed at the start of its
payload.
I'm sure there are other similar valid scenarios where attempting to
shrink a non writable packet is not "bad API usage".
If you want to shrink a writable packet, then maybe you don't even need
zero padding, because the buffer possibly already contains some defined
value, and the main reason of zero padding is avoiding reading
uninitialized data...
If i allocate a payload of size 1024, ultimately fill only 512 bytes,
then resize it to that value (typical scenario in lavf demuxers), if i
don't zero the bytes after the 512 offset then they will remain
uninitialized.
Regards,
Marton
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
