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 has been released! | Please read ffmpeg.org/developer.html#Code-of-conduct
<BBB>
jamrial: that does not solve it
<BBB>
jamrial: the dimensions are perfectly aligned
<BBB>
the buffer is just not padded
<BBB>
I agree with jannau that avcodec_align_dimensions2() feels like a bad fit, but that's just the naming. If we rename it to something that makes it explicit that it adds padding, it's fine with me
<jannau>
BBB: it adds 2 additional lines we can overread into
<BBB>
oh
<BBB>
ok
<BBB>
is it theoretically possible to get the same overflow on a different sample with slightly different dimensions?
<jannau>
avcodec_align_dimensions2 can't add padding. it just "aligns" width and height
<BBB>
ultimately, if we're on the last line, we can always overread outside the aligned size, right?
<BBB>
I wonder what's uglier, 2 extra lines, or stride_align+15
<BBB>
anyone saying ffmpeg is not hacky should participate in this discussion
<BBB>
jamrial: ok I guess
<jannau>
the overflow was only noticed because the dimensions align to a multiple of page size but the overread is always there. verified with valgrind and a change to the default allocator
<BBB>
jannau: does firefox use avcodec_align_dimensions2()?
<jannau>
no idea. I haven't looked at their code but from the ffmpeg documentation it's reasonable to expect that they do
<jamrial>
they may not. afaik they don't use our h264 decoder, for example
<Lynne>
jannau: it adds any padding that codecs require
<BBB>
it seems to use align_dimensions
<jamrial>
no, it doesn't add padding. it changes width and height adding a few lines where needed
<BBB>
jamrial: is it worth documenting *why* vp9 was added to that list?
<BBB>
I always find it confusing and weird that we add random codecs there but we don't explain why
<jamrial>
BBB: jannau says adding lines would not be ideal for this scenario, even if it does fix the issue
<jamrial>
if the asm can be fixed to not overread, then maybe that is enough and we don't need to make vp9 require extra lines to work around overreads
<BBB>
I think he said fixing the asm was not straightforward
<BBB>
it overreading by one byte was possible - he sent patches for that
<Lynne>
jamrial: exactly, users are supposed to follow this and allocate extra padding around the image
<BBB>
and then the overread would invoke emu_edge
<BBB>
but I (and I think he also) prefer padding (or aligning, it's essentially the same thing here) since that's moar fast
<BBB>
I have to dig really deeply in memory here, but I think I didn't understand back then that I could just update generic code, so I added emulation of edges *assuming* the aligned w/h are only 16
<BBB>
I should've just updated the generic code and align w/h by 64 for vp9
<BBB>
but ... ohwell
<BBB>
again, from memory
<BBB>
lemmesee if that makes sense... so long ago...
<jannau>
I don't see how extra alignment will help. I can just create a 4096x4096 video and the asm will still overread
<jannau>
the only thing which helps is to use the edge emulation code earlier when it it is on the last line
<BBB>
so the impl from line 810 doesn't call avcodec_align_dimensions2()?
<jannau>
no it calls av_image_get_buffer_size() and aligning line sizes to 16
<BBB>
hm... right
<BBB>
I guess that violates the api docs
<BBB>
(I know that doesn't help you ;) )
<jamrial>
no, the docs say "should call it", but it's basically a "do it or face the consequences"
<BBB>
ok
<BBB>
so s/should/must/ would be helpful
<kierank>
agreed
<jannau>
I think the 2 additional lines for h.264 in avcodec_align_dimensions2() is already surprising
<jannau>
I would say it's hard to blame some who doesn't call avcodec_align_dimensions2() but aligns width and height to 128
<BBB>
I agree
<BBB>
jannau: I don't think you're wrong
<BBB>
the problem that I think we risk running into is that our internal allocator is padding and aligning in undocumented ways, which is a recipe for bugs like this here
<BBB>
jannau: so part of what I'm trying to do is to make the padding/alignment expectations explicit
<BBB>
jamrial does that by saying "we'll add a magic number of lines and expect the user to call align_dimensions"
<BBB>
I did that by adding another magic number (15) + stride_align to the buffer size
<BBB>
they're both ugly and disgusting, but they're meant to document the internal behaviour, in some way
<BBB>
so external allocators can more easily reproduce it
<BBB>
or in jamrial's case, automatically, assuming they did use align_dimensions already (which firefox didn't :-/)
<jamrial>
i wrote the fuzzer implementation to not add such undocumented padding
<BBB>
right, but the fuzzer runs on x86 so it didn't find this bug :(
<jamrial>
yeah
<BBB>
I'm ok with your approach btw, jamrial
<BBB>
I can even live with jannau's patch, although we go from one ugly to another :)
<BBB>
(the one invoking emu_edge to protect against the overread)
<BBB>
it's all ugly in different ways
<BBB>
welcome to the never-ending world of pain called multimedia
<BBB>
hey I'm quoting wm4
Kei_N has joined #ffmpeg-devel
<jannau>
I don't see much advantage in doing it via avcodec_align_dimensions2(). we still need to fix the documentation to change the "should" into "must" if align_dimensions2() does surprising things. aligning the height for h264 to a multiple of 2 is surprising
<jamrial>
BBB: if jannau's asm fixes solve this, then lets go with that
<BBB>
ok
<BBB>
beastd: so I guess conclusion is we'll go with what's in jannau's branch right now, you can merge it if you want :) otherwise I'll do it tomorrow
<jannau>
the asm changes doesn't fix anything alone. The fix is to use the edge emulation code earlier
<BBB>
yes I know
<BBB>
it was a patchset of 3 patches
Kei_N_ has quit [Ping timeout: 265 seconds]
<BBB>
and apparently a v2 was missing from the list somewhere? do you want to send that? or should I merge from your branch?
<jannau>
the asm trades just makes the width 4 asm more complicated to ensure the overread is just 1 pixel and not 5
<BBB>
and the emu edge change is under #if ARCH_ARM? I thought that was what the v2 discussion was about?
<jannau>
I sent only the 3rd patch as v2 as the asm changes were unchanged, probably causing more confusing than reducing mails
<BBB>
I don't know that I have the v2
<BBB>
I see your email saying " I'll send an updated patch."
<BBB>
but I don't see the updated patch in my inbox
<Lynne>
jannau: its already a must in all but name
<jannau>
I don't see it either but I think I did sent it
<jannau>
BBB: I've sent now the full patchset as v2
<BBB>
ty
<BBB>
if you don't mind, I'll let it sit on the list for a day before merging it
<BBB>
but ok from my side and I'll merge tomorrow if not yet merged by someone else then
<jannau>
the git send-email command just sending Patch v2 3/3 is in my history, no idea why it wasn't sent
<BBB>
strange
<BtbN>
all 3 patches arrived just fine here
<jannau>
BBB: sure. no need to hurry from my side
<BBB>
jannau: tnx for the work on this
<jannau>
I just sent them. the missing mail is from before christmas
iive has quit [Quit: They came for me...]
thilo has quit [Ping timeout: 245 seconds]
thilo has joined #ffmpeg-devel
cone-932 has quit [Quit: transmission timeout]
<another|>
>1 mail in ffmpeg-devel == 1 shares
<another|>
this basically incentivizes spamming
<another|>
10 revisions of a 5 commit patchset that would normally have been 3 commits ~= 50 votes (+ discussion mails)
MyNetAz has quit [Remote host closed the connection]
MyNetAz has joined #ffmpeg-devel
DauntlessOne4 has quit [Ping timeout: 245 seconds]
System_Error has quit [Remote host closed the connection]
System_Error has joined #ffmpeg-devel
<kurosu>
it's so very far off from being used there or anywhere, but I wonder what is the status of vvc regarding overreads (and any api user less careful). Maybe this has been dealt with, but the cases for potential overreads are even larger here because of specific tools
ngaullier has quit [Remote host closed the connection]
ngaullier has joined #ffmpeg-devel
ngaullier has quit [Remote host closed the connection]
ngaullier has joined #ffmpeg-devel
ngaullier has quit [Remote host closed the connection]
ngaullier has joined #ffmpeg-devel
^Neo has joined #ffmpeg-devel
^Neo has quit [Changing host]
^Neo has joined #ffmpeg-devel
<BBB>
frankplow: I think it's more of an issue with older opts simd (or in this case: neon), so maybe vvc doesn't suffer from that because it has little neon and no mmx
ngaullier has quit [Remote host closed the connection]
ngaullier has joined #ffmpeg-devel
novaphoenix has quit [Quit: i quit]
novaphoenix has joined #ffmpeg-devel
LeaderForLife has quit [Quit: Client closed]
LeaderForLife has joined #ffmpeg-devel
ngaullier has quit [Remote host closed the connection]
ngaullier has joined #ffmpeg-devel
ngaullier has quit [Remote host closed the connection]
mkver has joined #ffmpeg-devel
LeaderForLife has quit [Quit: Client closed]
<kurosu>
frankplow: template matching (not sure the official name). iirc you do a "block" matching of the L shape around the block in a fixed neighbourhood around a MV using bilinear interpolation
<kurosu>
there may have been similar things around the various flavours of optical flow, but maybe that didn't get in and is instead in ecm now
jamrial has joined #ffmpeg-devel
cone-473 has joined #ffmpeg-devel
<cone-473>
ffmpeg James Almer master:8f6a1a06a692: avformat/iamf: use the correct layouts for Sound Systems B and C
<cone-473>
ffmpeg James Almer master:aba9fafee7fc: avutil/channel_layout: fix definition of 5.1.4 layout
<cone-473>
ffmpeg James Almer master:3fa70c03e4bf: avformat/iamf_writer: be more verbose when reporting an input layout is invalid
<cone-473>
ffmpeg James Almer master:da9dcaba69da: avutil/channel_layout: add a 5.1.2 layout using side channels
<cone-473>
ffmpeg James Almer master:547408ce1d90: avformat/iamfdec: swap back and side streams if both are present
<cone-473>
ffmpeg James Almer master:c089c158d6c5: avformat/iamf: document the expandable channel layouts
<cone-473>
ffmpeg James Almer master:fb59995b88c9: avformat/flvdec: set Opus sample rate
<cone-473>
ffmpeg James Almer master:873a34c12986: avcodec/opus/parser: set sample rate
<cone-473>
ffmpeg James Almer master:c187dd88de7d: avcodec/opus/parser: reindent after the previous commit
<cone-473>
ffmpeg James Almer master:4bf784c0e561: avformat/dump: print only the actual streams in a tile grid group
ngaullier has quit [Remote host closed the connection]
cone-938 has joined #ffmpeg-devel
<cone-938>
ffmpeg Michael Niedermayer release/7.1:811fce437ecc: avcodec/ffv1enc: Slice combination is unsupported
<cone-938>
ffmpeg Michael Niedermayer release/7.1:e4538bc88825: avcodec/ffv1enc: 2Pass mode is not possible with golomb coding
<cone-938>
ffmpeg Michael Niedermayer release/7.1:76ddc3f1e1e6: avcodec/ffv1: Store and reuse sx/sy
<cone-938>
ffmpeg Michael Niedermayer release/7.1:ba89c5d1ebc6: avformat/mov: Avoid overflow in dts
<cone-938>
ffmpeg Michael Niedermayer release/7.1:3f76a333172c: avcodec/ffv1enc: Correct error message about unsupported version
<cone-938>
ffmpeg Michael Niedermayer release/7.1:631976108ca8: avformat/matroskadec: Check desc_bytes so bits fit in 64bit
<cone-938>
ffmpeg Michael Niedermayer release/7.1:d870febf88c7: avcodec/ffv1enc: Prevent generation of files with broken slices
<cone-938>
ffmpeg Michael Niedermayer release/7.1:2a39eeb8deb3: avcodec/ffv1enc: Move slice termination into threads
<cone-938>
ffmpeg Michael Niedermayer release/7.1:26fc4bf42c19: avcodec/ffv1dec: Fix end computation with ec=2
<cone-938>
ffmpeg Michael Niedermayer release/7.1:4ca3f5102f5d: avcodec/ffv1enc: Fix RCT with RGB64
<cone-938>
ffmpeg Michael Niedermayer release/7.1:7043ef682854: avcodec/ffv1: RCT is only possible with RGB
<cone-938>
ffmpeg Michael Niedermayer release/7.1:9f8bd56e4f57: avcodec/ffv1: add a named constant for the quant table size
<cone-938>
ffmpeg Michael Niedermayer release/7.1:a2666675bf74: avcodec/rangecoder: only perform renorm check/loop for callers that need it
<cone-938>
ffmpeg Michael Niedermayer release/7.1:2ab2803944ca: swscale/rgb2rgb_template: Fix ff_rgb24toyv12_c() with odd height
<cone-938>
ffmpeg Michael Niedermayer release/7.1:562af93025dc: swscale/output: used unsigned for bit accumulation
<cone-938>
ffmpeg Michael Niedermayer release/7.1:6b0204781126: avformat/mxfdec: Fix overflow in midpoint computation
<cone-938>
ffmpeg Michael Niedermayer release/7.1:0288fedf1815: avformat/mpegts: Initialize predefined_SLConfigDescriptor_seen
<cone-938>
ffmpeg Michael Niedermayer release/7.1:ded98a0919a0: INSTALL: explain the circular dependency issue and solution
<cone-938>
ffmpeg Michael Niedermayer release/7.1:241e87afa4c5: avformat/rpl: check channels
<cone-938>
ffmpeg Michael Niedermayer release/7.1:e2394166bfa9: avformat/mccdec: Initialize and check rate.den
<cone-938>
ffmpeg Michael Niedermayer release/7.1:1f1b309f9e18: avformat/nistspheredec: Clear buffer
<cone-938>
ffmpeg Michael Niedermayer release/7.1:445065e23a7a: avformat/ilbc: Check avio_read() for failure
<cone-938>
ffmpeg Michael Niedermayer release/7.1:c7aa0c4ecd28: avformat/vividas: Check avio_read() for failure
<cone-938>
ffmpeg Michael Niedermayer release/7.1:4930dd91c6b4: doc/infra: Document gitolite
<cone-938>
ffmpeg Michael Niedermayer release/7.1:f9901306ba43: doc/infra: Document trac backup system
<cone-938>
ffmpeg Michael Niedermayer release/7.1:d9687e6156a8: doc/developer: Document relationship between git accounts and MAINTAINERS
<cone-938>
ffmpeg Michael Niedermayer release/7.1:3417e955c3c6: avformat/icodec: fix integer overflow with nb_pal
<cone-938>
ffmpeg Michael Niedermayer release/7.1:49e4c1717ffb: avcodec/mjpegdec: Disallow progressive bayer images
<cone-938>
ffmpeg Michael Niedermayer release/7.1:8ac2375b71fa: MAINTAINERS: Remove Guillaume Poirier and Romain Dolbeau
<cone-938>
ffmpeg Michael Niedermayer release/7.1:3c8b588f3c11: MAINTAINERS: Lauri is still available but is really low on time nowadays
<cone-938>
ffmpeg Michael Niedermayer release/7.1:173a978b9d8c: avcodec/h2645_parse: Ignore NAL with nuh_layer_id == 63
<cone-938>
ffmpeg Michael Niedermayer release/7.1:f9f483573161: swscale/slice: clear allocated memory in alloc_lines()
<cone-938>
ffmpeg Michael Niedermayer release/7.1:851bc9927d98: avformat/dxa: check bpc
<cone-938>
ffmpeg Michael Niedermayer release/7.1:048a545e316e: avcodec/eatgq: Check bytestream2_get_buffer() for failure
<cone-938>
ffmpeg Michael Niedermayer release/7.1:9285b9314353: avformat/qcp: Check for read failure in header
<cone-938>
ffmpeg Michael Niedermayer release/7.1:82d45cb004a9: avcodec/hevc/hevcdec: initialize qp_y_tab
<cone-938>
ffmpeg Michael Niedermayer release/7.1:028391aa5809: swscale/swscale_unscaled: Fix odd height with nv24_to_yuv420p_chroma()
<cone-938>
ffmpeg Michael Niedermayer release/7.1:886dd058fe13: avcodec/ilbcdec: Initialize tempbuff2
<cone-938>
ffmpeg Michael Niedermayer release/7.1:bc8248d07aa5: avcodec/webp: Check ref_x/y
<cone-938>
ffmpeg Michael Niedermayer release/7.1:828569c0d0ef: avcodec/aac/aacdec_usac: Clean ics2->max_sfb when first SCE fails
<cone-938>
ffmpeg Michael Niedermayer release/7.1:b44488042243: avcodec/aac/aacdec_usac: Dont leave type at a invalid value
<cone-938>
ffmpeg Michael Niedermayer release/7.1:52461e7e8b54: avutil/timecode: Avoid fps overflow in av_timecode_get_smpte_from_framenum()
<cone-938>
ffmpeg Michael Niedermayer release/7.1:63505308068a: tools/target_dec_fuzzer: Adjust Threshold for indeo5
<cone-938>
ffmpeg Michael Niedermayer release/7.1:364eb21d2a34: tools/target_dec_fuzzer: Adjust threshold for MVC1
<cone-938>
ffmpeg Michael Niedermayer release/7.1:5e17ff811ae8: tools/target_dec_fuzzer: Adjust threshold for EACMV
<cone-938>
ffmpeg Michael Niedermayer release/7.1:1cb5caeb5be0: avformat/matroskadec: Check pre_ns for overflow
<cone-938>
ffmpeg Michael Niedermayer release/7.1:8ae93fdc426a: avcodec/utils: Fix block align overflow for ADPCM_IMA_WAV
<cone-938>
ffmpeg Michael Niedermayer release/7.1:a5ce14389510: avformat/mov: dereference pointer after null check
<cone-938>
ffmpeg Michael Niedermayer release/7.1:a82139d0e690: avcodec/aac/aacdec: Free channel layout
<cone-938>
ffmpeg Michael Niedermayer release/7.1:cac9112bf35c: avformat/mlvdec: Check avio_read()
<cone-938>
ffmpeg Michael Niedermayer release/7.1:615f29e301fd: avformat/rpl: Fix check for negative values
averne has quit [Remote host closed the connection]
darkapex has quit [Remote host closed the connection]
darkapex has joined #ffmpeg-devel
<fflogger>
[editedticket] winterlint_: Ticket #4591 ([avformat] attachments mjpeg are discovered wrongly as video stream and therefore h264 encoding is broken) updated https://trac.ffmpeg.org/ticket/4591#comment:12
averne has joined #ffmpeg-devel
averne has quit [Client Quit]
averne has joined #ffmpeg-devel
System_Error has quit [Ping timeout: 264 seconds]
System_Error has joined #ffmpeg-devel
witchymary has quit [Remote host closed the connection]
witchymary has joined #ffmpeg-devel
___nick___ has quit [Ping timeout: 265 seconds]
cone-938 has quit [Quit: transmission timeout]
witchymary has quit [Remote host closed the connection]
witchymary has joined #ffmpeg-devel
cone-141 has joined #ffmpeg-devel
<cone-141>
ffmpeg James Almer master:37155d68ec62: avcodec/opus/parser: set duration when complete frames are fed
HarshK23 has quit [Quit: Connection closed for inactivity]
ngaullier has quit [Remote host closed the connection]
ccawley2011 has quit [Ping timeout: 248 seconds]
mkver has quit [Ping timeout: 252 seconds]
ccawley2011 has joined #ffmpeg-devel
Riviera has joined #ffmpeg-devel
darkapex has quit [Remote host closed the connection]
darkapex has joined #ffmpeg-devel
deus0ww has quit [Ping timeout: 265 seconds]
deus0ww has joined #ffmpeg-devel
mkver has joined #ffmpeg-devel
Coinflipper has quit [Quit: ]
<cone-141>
ffmpeg Janne Grunau master:430c38f698a6: aarch64: vp9mc: Load only 12 pixels in the 4 pixel wide horizontal filter
<cone-141>
ffmpeg Janne Grunau master:f3662562156c: arm: vp9mc: Load only 12 pixels in the 4 pixel wide horizontal filter
<cone-141>
ffmpeg Janne Grunau master:060464105bdc: vp9: recon: Use emulated edge to prevent buffer overflows
ccawley2011 has quit [Read error: Connection reset by peer]
Coinflipper has joined #ffmpeg-devel
<wbs>
BBB: I guess ^ should be backported too? (I also recently merged some aarch64 h264 asm fixes, I'll try to backport them when I have time, unless someone else beats me to it)
osvein has quit [Remote host closed the connection]
ngaullier has joined #ffmpeg-devel
System_Error has quit [Remote host closed the connection]
<fflogger>
[newticket] breunigs: Ticket #11396 ([undetermined] filter_complex pipeline "segment;guided;concat" sometimes fails assertion) created https://trac.ffmpeg.org/ticket/11396
System_Error has joined #ffmpeg-devel
<fflogger>
[newticket] spooky: Ticket #11397 ([undetermined] Empty file version information in Windows File explorer.) created https://trac.ffmpeg.org/ticket/11397
System_Error has quit [Remote host closed the connection]