[FFmpeg-devel] [PATCH 2/5] ffmpeg: use the write_uncoded_frame() API.

[Lukasz Marek]

On 25.01.2014 17:13, Nicolas George wrote:
> Le sextidi 6 pluviôse, an CCXXII, Michael Niedermayer a écrit :
>> On Sat, Jan 25, 2014 at 02:47:38PM +0100, Lukasz Marek wrote:
>>> I haven't follow this discussion deeply, but is it blocking to merge
>>> first patch from the set: [PATCH 1/5] lavf: add
>>> write_uncoded_frame() API.?
>> no, just needs nicolas/the author to ask for it to be merged
>
> IMHO, patch #1 can not be merged until the current discussion on the best
> design for the API is finished, since patch #1 actually implements the API.
>
> Lukasz, as the author of several devices and a player, can give his advice
> on this. Ramiro too.
>
> I can summarize the current discussion, I hope I am not over-simplificating:
>
> My current proposal is this:
>    - new method for the muxers;
>    - new public function with an AVFrame* argument;
>    - internally to lavf/mux.c, disguise/wrap the AVFrame into an AVPacket;
>    - internally to ffmpeg.c, disguise/wrap the AVFrame into an AVPacket.
>
> Michael's argument is this:
> since ffmpeg.c needs the AVFrame-disguised-as-AVPacket pattern, other
> applications will need too, and therefore the API should be just that, which
> is very similar to the RAWPICTURE special case and Ramiro's original
> proposal.

I don't understand what you mean by "the API should be just that" :)
Anyway some thoughts below.

> My opinion concerning that is this:
> ffmpeg.c is quite unusual and should probably not use this API at all (it is
> meant for players, not transcoders). Players do not perform complex tasks on
> AVPacket structures, and therefore do not need this frame-as-packet hack.
>
> I would gladly hear the opinion of someone else who is trying to get lavd
> suitable for actual output.
>
> Regards,

I haven't reviewed your patches deeply, so don't ask me about details, 
and I may miss many things, but one remark I had from the beginning is 
that it seems to be much more complex than it can be.

I expected it to be a single function like:

int av_format_write_frame(AVFormatContext *s, const AVFrame *frame);

Function just check if format supports it and it is passed directly to 
output device. After quick look all required params are available.

I assume muxers cannot omit all crazy staff that is done in 
av_write_frame just like that.
I don't have opinion about leaving this hackery option or not.

I checked Ramiro's patch and I don't understand how it is related TBH.
I don't understand Ramiro's patch at all TBH :)
I assume Ramiro tries to remove memcpy from ffmpeg.c, but how it can be 
used outside ffmpeg.c?
I'm quite tired already and it is to much to read.

Coming back to your solution, in my opinion it is not forth the effort 
to support transcoding. Reasons:
- Transcoding is usually not real-time operation.
- I guess transcoding to uncompressed form is rare case
- Transcoding HD streams to uncompressed form is even more stupid and 
that's the case it would benefit the most (more data to be copied for no 
special reason)
- I also believe it is a rare to do transcoding on mobile devices, 
embedded systems etc, where these 10% cpu really do matter.

But I don't want to be an ignoramus, faster transcoding is always better 
than slow one.

Unfortunately I don't know ffmpeg well at this level so cannot propose 
anything better than simple opinion.

-- 
Best Regards,
Lukasz Marek

Microsoft isn't evil, they just make really crappy operating systems. - 
Linus Torvalds