On 4/19/2020 4:32 PM, Marton Balint wrote: > > > On Sun, 19 Apr 2020, James Almer wrote: > >> On 4/19/2020 3:26 PM, Marton Balint wrote: >>> >>> >>> On Sun, 19 Apr 2020, James Almer wrote: >>> >>>> On 4/19/2020 1:01 PM, Marton Balint wrote: >>>>> >>>>> >>>>> On Sun, 19 Apr 2020, James Almer wrote: >>>>> >>>>>> Process input data as soon as it's fed to av_bsf_send_packet(), >>>>>> instead of >>>>>> storing a single packet and expecting the user to call >>>>>> av_bsf_receive_packet() >>>>>> in order to trigger the decoding process before they are allowed to >>>>>> feed more >>>>>> data. >>>>>> >>>>>> This puts the bsf API more in line with the decoupled decode API, >>>>>> without >>>>>> breaking existing using it. >>>>> >>>>> Well, previously it was assumed that av_bsf_send_packet() is never >>>>> returning AVERROR_INVALIDDATA, that is no longer the case. That >>>>> matters >>>>> because current code in mux.c ingnores av_bsf_receive_packet() errors >>>>> but not av_bsf_send_packet() errors. >>>> >>>> The doxy states av_bsf_send_packet() can return an error, even if up >>>> till now it hasn't. >>>> >>>> Also, as i stated in the addendum below the Signed-off line, current >>>> users following the recommended API usage (one call to >>>> av_bsf_send_packet(), then several to av_bsf_receive_packet()) will see >>>> no changes at all. So if you did that and expected no errors from >>>> av_bsf_send_packet() in your mux.c code, you'll still get none. >>>> >>>>> >>>>> Unless there is some benefit I am not seeing, I'd rather keep >>>>> things as >>>>> is. Sorry. >>>> >>>> The benefit is relaxing the current constrains of the API for new users >>>> (including mux.c if you want to take advantage of it), >>> >>> How is this making things easier for the API user? After the patch, the >>> framework can - in most cases - buffer 2 packets instead of 1. So what? >>> User app still have to implement that if it gets EAGAIN in send_packet, >>> it has to call receive_packet and vica versa. Unless it uses the >>> "recommended" practice, where it can assume success of send_packet for a >>> completetly drained bsf. >> >> The difference is that i can now call send_packet as many times as the >> underlying bsf lets me, > > That is the main difference, yes, thanks. > >> much like the decode send_packet, which allows >> me to build the loop in many different ways and not just the currently >> enforced "Send one packet, receive packets until drained" one. >> >>> >>> You say that the API should be the same as for encode/decode. Well, it >>> already is. >> >> It certainly isn't. Unlike the decode API, I can't feed the >> av1_frame_merge bsf all the packets it needs before it has one ready for >> output. I need to feed them one, interleaved with at least one >> receive_packet call that will tell me that i need send more before it >> can output anything. > > I just don't see why this is an issue? Does it measurably help > performance if the number of API calls are reduced which return EAGAIN? > Or does this have some other benefit?
More freedom for the API user to write their loops by removing a strict constrain, and making all our decouple input/output APIs behave as similar as possible (Having them behave exactly the same is impossible without breaking the bsf API, making it return EOF instead of 0 on subsequent flush packets). Just that. > >> >>> Decode/encode API makes no guarantees about the number of >>> possibly buffered frames/packets. It only says that one of successive >>> send/receive calls must succeed and not return EAGAIN. And that is true >>> with the current BSF implementation. >>> >>> You can remove the comment that "the filter must be completely drained", >>> or change it to that it is recommended to completetly drain the bsf. But >>> the code need no changes, it will still be API-compatible with the >>> decode API. It just uses internally a stricter scheme, and I like that >>> it does. >> >> I'm removing the stricter scheme without altering the workflow of users >> of said strict scheme. It's not breaking your mux.c code, right? > > Right. > >> Or at least it's not meant to if you followed the current strict >> guidelines. So what exactly are you against of? > > I am not against it anymore, feel free to go ahead. I am just still not > sure about the general usefulness. Will wait a bit before pushing in any case. Thanks. > > Thanks, > 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".