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.0 has been released! | Please read ffmpeg.org/developer.html#Code-of-conduct
kurosu has quit [Quit: Connection closed for inactivity]
<SuperFashi>
I was recently fixing a bug and bisected to this commit d383ae43c266b160348db04f2fd17ccf30286784. This commit definitely seems buggy, it does no reset `codec->extradata_size` and it just removes extradata which the codec might need.
<SuperFashi>
maybe there were originally reasons for this but I cannot seem to find the reason why this commit exists
<JEEB>
well that was made by jamrial at least
<JEEB>
so at least you are in a channel where that person exists :)
<JEEB>
anyways, feel free to note the case that is now broken. although yes, not resetting extradata size sounds funky.
<jamrial>
yeah, needs to reset extradata_size, my bad
<SuperFashi>
however, are you sure that it's okay to just remove the extra data? i thought that might be needed by the codec to keep context
<jamrial>
these functions are not meant to mix data from the source and the data already in the destination
<SuperFashi>
at least it doesn't work with the demuxer i wrote before this commit, which does not use extra data from AVCodecParameters and just contains it in the packet. The decoder seems to extract PPS and SPS and put it into extradata
<SuperFashi>
feels like the bug is triggered when `need_context_update` is set midstream
<SuperFashi>
yeah, once i remove setting `need_context_update` it works but the codec parameter is not updated correctly anymore
Krowl has quit [Read error: Connection reset by peer]
<jamrial>
SuperFashi: if the extradata is inband, is it not present on every keyframe?
<SuperFashi>
not necessarily true for the stream i'm working with
System_Error has quit [Remote host closed the connection]
System_Error has joined #ffmpeg-devel
<jamrial>
ok, so my commit is not ok, documentation says "Fields in codec that do not have a counterpart in par are not touched."
<jamrial>
it's only avcodec_parameters_from_context which does not merge fields
<jamrial>
or maybe that just means fields not in avcodecparameters, and not fields not filled in the source par
pmozil has quit [Remote host closed the connection]
darkapex has quit [Remote host closed the connection]
darkapex has joined #ffmpeg-devel
rvalue has quit [Read error: Connection reset by peer]
rvalue has joined #ffmpeg-devel
alien_lappy has joined #ffmpeg-devel
<alien_lappy>
So, i'm working on upstreaming the RPI4 v4lm2m codec driver; and would it be possible for someone to put hwaccel in the v4l2m2m modules?
Raz- has quit [Ping timeout: 245 seconds]
<alien_lappy>
Also, would it be possible to allow for no B frames? instead of having an empty list of B-frames? the RPI4 hw does not have B-frame support, but we have to advertise it in order for ffmpeg to be able to use it; according to spec we could just have no mention of it
<JEEB>
FFmpeg's decoder or hwaccel infra don't have limitations wrt b-frames being or not being there :)
<JEEB>
so whatever your module would do is up to you
<JEEB>
as for hwaccel, I don't think anyone else is caring enough about it to make a hwaccel :) not sure if the stateless interface patches in the forks have a hwaccel
<JEEB>
so outside of basing on someone else's code, you seem to be the one caring the most :)
<BtbN>
I don't think the m2m stuff _can_ be a normal hwaccel? It has its own parser, doesn't it?
<JEEB>
it's the "stateful" interface, yea.
<alien_lappy>
I have no idea about this stuff, I can't really get into another project, tbh; but in any case, if i'm playing without codec specified, it won't use the v4l2m2m (hwcodec), it'll just use the regular h264 (software)...
<alien_lappy>
the B-frames is unrelated
<BtbN>
That's just how it works
<BtbN>
the "hwaccel" you're talking about is that the native decoders can offload to various hwaccels, while still maintaining state itself
<BtbN>
v4l is no such interface
kurosu has joined #ffmpeg-devel
<JEEB>
but I thought it'd be similar to videotoolbox, even with the stateful interface
<JEEB>
but if not matchable, but eh
<JEEB>
s/but/then/
<JEEB>
alien_lappy: various API clients do have "fake hwaccel" logic where they instead add _IDENTIFIER to the end, which is how f.ex. mpv can utilize these things automagically, but ffmpeg cli itself has no such thing, you need to specify the decoder manually
<BtbN>
It would probably be a worthwhile addition for ffmpeg cli to support specifying multiple decoders, and then use the first that works
<alien_lappy>
man, i just want people to use the hw if it's availlable
<alien_lappy>
about B frames: "FFmpeg insists on trying to set V4L2_CID_MPEG_VIDEO_B_FRAMES to 0, and generates an error should it fail. As our encoder doesn't support B frames, add a stub handler for it to silence FFmpeg."
<alien_lappy>
this is about the control
<alien_lappy>
we need to have the control or ffmpeg can't use the driver, even though specs says we do not need to have this control
<BtbN>
sounds more like a case of "access quirky hardware via a generic interface"
<BtbN>
Nvidia does the same stuff on their Jetson boards, and when they wanted to add an entire copy of the v4l2 video stuff with their quirks, it was rejected
<alien_lappy>
so, you mean that this codec driver is useless they shouldn't have gone through v4l_m2m?
<BtbN>
If there is some API that attempts to be generic, but you then need a separate client implementation for each actual bit of hardware using that API, the API has failed to do its job
<BtbN>
well, they should implement the driver so that a generic client can work
<BtbN>
if that's not possible, the whole API is kinda busted in itself
<alien_lappy>
well, it's not that it doesn't work
<alien_lappy>
gstreamer and ffmpeg work
___nick___ has quit [Ping timeout: 256 seconds]
<alien_lappy>
it's just that the b_frames is unneeded according to spec and gstreamer doesn't need it, but for ffmpeg we do
<JEEB>
oh, encoder? then that just needs more feature check logic (hopefully v4l2 provides interfaces to check whether a certain feature is supported by driver)
<alien_lappy>
atm, we implement the control, just to have it return 0
<BtbN>
An encoder will never be automagically selected though, you always need to be specific
<alien_lappy>
kinda wasteful
<alien_lappy>
the codec is a encoder and decoder
<JEEB>
yes, but those are two separate parts of it
<JEEB>
and then only error out if avcodec user has explicitly set b-frames to a wanted number, but the driver doesn't support it
<JEEB>
hopefully v4l2 does let you tell that "hey this driver doesn't implement this thing"
<JEEB>
like that CID_MPEG_VIDEO_B_FRAMES
<alien_lappy>
for me personally, if i use ffmpeg, i need to specify the driver to decode it, or it'll use software decoder
<BtbN>
That's how it works, yes
<JEEB>
yes because it's a separate decoder at the moment
<BtbN>
The default is the native decoder
<BtbN>
If you want another decoder, you need to specify it
<JEEB>
thus is why I noted that if you can fit it somehow into the hwaccel interfaces, that fixes that issue :P
<alien_lappy>
however, would the default logic not be to prefer hw if it's available? or is that not how ffmpeg works?
<JEEB>
libavcodec picks the first decoder for the format
<BtbN>
How would it possibly know which hw exists and is in working state?
<JEEB>
and no, by default hwaccels aren't utilized either
<alien_lappy>
ic
<alien_lappy>
well, ok then
<JEEB>
(with hwaccels you gain metadata and just setting it with -hwaccel THING with ffmpeg cli, instead of having to set each format's decoder separately)
<JEEB>
but without hwaccel defined things will do the default behavior of the library, which is normal swdec
<Lynne>
alien_lappy: you mean that b-frames are not supported for encoding, right?
<alien_lappy>
the underlying hw does not support b-frames
<Lynne>
you keep talking about hwaccel but that term is only used for encoders
<Lynne>
wait, you mean they're not supported for DEcoding too?
<alien_lappy>
hwaccel is not used for decoders?
<Lynne>
mistype, hwaccel is a term only used for decoders
<alien_lappy>
oh, ok
<alien_lappy>
the underlying hw of the RPI4 does encoding and decoding
<Lynne>
yeah...?
<alien_lappy>
the hw doesn't support B-frames anywhere
<JEEB>
Lynne: he's talking about both
<JEEB>
he separately complains about having to specify the decoder
<alien_lappy>
but we need to implement a B-frame control because otherwise ffmpeg can't work
<JEEB>
and separately then brings up the encoder trying to set a value for bframes
<JEEB>
alien_lappy: I hope you have already checked this, but I hope that there is a function that lets an API user check whether a control is supported or not
<JEEB>
because I believe that is the way to go
<alien_lappy>
there's 2 issues, one is annoying for me, the other is to reduce code in the kernel driver
<Lynne>
encoding-wise, it's not an issue if b-frames are not supported
<Lynne>
decoding-wise, the fuck?
<alien_lappy>
JEEB: i have not checked this
<Lynne>
if an h264 decoder doesn't support b-frames, it's not a spec-compliant decoder and they can't say they support h264 decoding
<JEEB>
and then the logic would be on the v4l2 encoder to check whether the feature is supported, and only poke the control if it's supported. and error out if the API user specifically requested nondefault amount of bframes
<JEEB>
Lynne: the b-frame stuff is specifiaclly for the encoder
<JEEB>
the v4l2 encoder seems to always touch the b-frame interface and sets zero
<JEEB>
the rpi encoder doesn't implement this interface, and errors out
<JEEB>
thus my recommendation to alien_lappy was to check whether there is a feature check interface so that the logic can be properly handled :P
<Lynne>
okay
<JEEB>
or how is an API client supposed to know what a specific driver supports
<alien_lappy>
lemme get the exact commit
<alien_lappy>
that i'm trying to get rid of
<JEEB>
or if the way to inform an API client that the v4l2 driver doesn't support a specific interface is by the return value, then the error check would have to keep an eye on whether the FFmpeg API user actually requested a specific amount of XYZ or not. but if you know about v4l2, you should know what the logic behind feature checking should be.
<JEEB>
I hope you can understand what I'm speaking about :P
<JEEB>
because I don't expect people who write v4l2 encoder code to check for each specific driver in their code, and only call certain interfaces based on their logic of "X years ago this driver only implemented this, this and this"
<JEEB>
I mean, I know the line in the v4l2 encoder module, so that is why I'm telling how that can be improved
<JEEB>
it's just that whomever implemented that module at the time just added whatever that person thought was safe to call
<alien_lappy>
I get that, i was hoping someone could make a quick fix on this and I can move on, cause i have 21 more issues to fix on this driver before it can be upstreamed/unstaged
<alien_lappy>
and i'm not paid for any of this
<JEEB>
I'm not either
<alien_lappy>
that is fine
<JEEB>
in any case, check how v4l2 *thinks* this should be done
<JEEB>
how is an API client of v4l2 supposed to check what the module behind the scenes supports
<JEEB>
because then the encoder module can properly handle this.
<alien_lappy>
yeah, i'll check on that part, I got a comment from a person (who is biased, cause he's a gstreamer maintainer), that this could be removed if I get this fixed with ffmpeg peeps
* alien_lappy
sighs , more work, i'll note it down
<JEEB>
well yes, if there is a way to check for features then of course the encoding module should handle this correctly
<JEEB>
and to not just fail even if the FFmpeg API client didn't request b-frames
<JEEB>
and yes, the implementation of that driver could be removed if there was a proper way of handling this in the v4l2 API
<JEEB>
which is why I'm pointing towards that, if it was not clear yet
<alien_lappy>
i get it, np
<JEEB>
ok, great
<alien_lappy>
thanks for all the help
<alien_lappy>
i need to look this up, but iinm available controls are advertised