michaelni changed the topic of #ffmpeg-devel to: Welcome to the FFmpeg development channel | Questions about using FFmpeg or developing with libav* libs should be asked in #ffmpeg | This channel is publicly logged | FFmpeg 7.1.1 has been released! | Please read ffmpeg.org/developer.html#Code-of-conduct
<michaelni>
mkver, seems forgotten AV_INPUT_BUFFER_PADDING_SIZE, ill add it unless you already did
<mkver>
I have not yet done so.
<mkver>
It also seems crazy: This new extradata seems to have a separately allocated setup array.
<mkver>
How is non-flat extradata supposed to work?
<mkver>
And this vorbis_new_extradata is even part of a public header, despite being in violation of our naming conventions.
<mkver>
And vorbis_parser.h breaks checkheaders now (missing stddef.h).
<mkver>
Seems like a revert is in order.
<jamrial>
second patchset pushed without a proper review in a month...
<mkver>
I actually thought that michaelni had thoroughly reviewed it.
<mkver>
Hasn't he found issues in earlier iterations?
<michaelni>
i thought lynne reviewd it ;)
<mkver>
And Lynne points to ...
<jamrial>
but yeah, non-flat array and out of place structs is a big no
<mkver>
I am actually surprised that they would use new-extradata sidedata here. Given that our parsing API does not handle AVPackets, side data has a tendency to end up on the wrong packet or be lost altogether (unless it is a "complete frames" parser).
MisterMinister has quit [Remote host closed the connection]
<michaelni>
the non flat extradata was added in v6 of the patchset
<michaelni>
patch to revert is on ML
<michaelni>
mkver, jamrial ^
<mkver>
courmisch: https://fate.ffmpeg.org/report.cgi?time=20250531001843&slot=rv64gcvb-linux-gnu-gcc already contained 75960ac2708, which should have fixed the crashes with vsynth3-asv[12] by switching to get_pixels_unaligned. (That commit fixed it with arm under qemu for me.) So why does it still crash? If something is wrong with the assembly, why does checkasm (which uses a constant misalignment of one) pass?
<cone-427>
ffmpeg Michael Niedermayer master:848ceb1329cb: Revert "ogg/vorbis: implement header packet skip in chained ogg bitstreams."
<jkqxz>
Why does av_bprint*() not return an error code?
<jkqxz>
(Is it used in any local-security-sensitive context where an attacker could cause a transient out-of-memory and make it silently fail an allocation?)
<mkver>
You are supposed to check whether the bprint is complete before you actually use the buffer.
<jamrial>
yeah, you're supposed to call av_bprint_is_complete(), or just av_bprint_finalize() if you're done
<mkver>
av_bprint_finalize() does not tell you whether the string is truncated.
<jamrial>
the why does the doxy state "Test if the print buffer is complete (not truncated)."?
<mkver>
I pondered to add a variant of it that errors out when the string is truncated, to simplify error handling, but never did.
<mkver>
That's the doxy for is_complete, not finalize.
<jamrial>
oh, my bad, thought we were talking about is_complete
<jamrial>
but well, finalize would return enomem on truncation
<jkqxz>
Why is AVBPrint.str not hidden from the API?
<mkver>
Because the whole API is designed to avoid allocations (it has a built-in small-string optimization) and so it needs to be a type with known sizeof that can be put on the stack/in structures.
<jkqxz>
Well, yes, but it could at least call the field IF_YOU_USE_THIS_FIELD_THEN_YOUR_PATCH_IS_BAD_AND_YOU_ARE_A_BAD_PERSON?
<jkqxz>
Which would hopefully get the point across.
<jkqxz>
It seems to be used all over the place.
<mkver>
What exactly would be the point of avoiding these direct accesses? Do you want getters instead?
<jkqxz>
So direct access is meant to be allowed?
<mkver>
Yes.
<mkver>
E.g. the doxy for finalize states that the len and size fields are still valid afterwards.
<jkqxz>
But if truncation mattered then you must have checked av_bprint_is_complete() touching str.
<mkver>
Yes, of course. (But notice that the string is always zero-terminated, even when truncated.)
<jkqxz>
Yes, effectively snprintf behaviour.
<jamrial>
most cases of direct str use are AV_BPRINT_SIZE_AUTOMATIC users, which expect the final string to be well within the stack limit (1kb), and may not even bother to call finalize
<jkqxz>
Doing this to construct a filename or the argument to system() without checking is_complete() is definitely a disaster in any case.
witchymary has joined #ffmpeg-devel
<jkqxz>
mkver: Making patch sets with a title taken from the first patch and which requiring looking at the attachments to determine what they are related to is slightly unhelpful.
<jkqxz>
Can I suggest using --cover-letter and pasting the 0/N text into the email, and also giving a slightly more helpful title?
<jkqxz>
Some of these are already applied. Which am I looking at?
<mkver>
From patch #4 onward.
<mkver>
Wait, five onward
<fflogger>
[newticket] Noki0100: Ticket #11618 ([ffmpeg] hwupload filter fails with "Cannot allocate memory" for VA-API on AMD RX 7900 XT (Navi 31) preventing H.264/HEVC hardware encoding initialization.) created https://trac.ffmpeg.org/ticket/11618
ccawley2011 has quit [Ping timeout: 272 seconds]
ccawley2011 has joined #ffmpeg-devel
ccawley2011 has quit [Read error: Connection reset by peer]