[FFmpeg-devel] [PATCH] avfilter: take_samples: do not directly return frame when samples are skipped

[Nicolas George]

Le decadi 30 floréal, an CCXXV, Muhammad Faiz a écrit :
> Every patch that can fix the crash of
> ffmpeg -i clip.wav -af alimiter -c:a mp3 -b:a 128k -ar 48k -f null -
> can claim that it fixes ticket 6349.
> 
> Other cases should be in separate tickets.

No, you are confusing the bug with its symptom. Chewing carefully does
not CURE a dental cavity, it only works around it. The correct cure is
to go to the dentist.

This is the same here: you are working around the bug, but not fixing
it. The correct fix is there:
https://patchwork.ffmpeg.org/patch/3694/
The project having become such an inhospitable place, I will no longer
be working on it, but please feel free to take it over, pushing it as is
or reworking it. Anyway, I will be rejecting any patch that pretends to
"fix" this ticket with changes in lavfi.

> IMHO, the solution is to document it properly to not mix
> ff_inlink_consume_samples with ff_inlink_consume_frame, similar to
> av_buffersink_get_frame vs av_buffersink_get_samples.

As I said, I do not like the idea of limiting the API if it is not
absolutely necessary. And I think it is not. What about this:

If samples_skipped is set, then ff_inlink_consume_frame() can call
ff_inlink_consume_samples() with the exact number of samples to force it
to realign the frame at this point. (Of course, it requires disabling
the case that does the opposite.)

Would you look carefully at the code and tell me I did not miss
something that would make it harder than it looks.

If not, if you confirm it looks feasible, then I think this patch can go
in almost as is: we do not need to actually implement it before it is
required, that would be a waste of time, just add a comment near the
assert to tell it is the plan. (And of course, remove "fix" from the
commit message.)

Now, I am not entirely sure that I understand correctly how this patch
takes into account the number of samples skipped. I would like to look
at it more carefully, but as you can see, Paul is rushing me.

Regards,

-- 
  Nicolas George
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20170520/96da0084/attachment.sig>