[FFmpeg-devel] add MJPEG support into RTP output

[Luca Abeni]

Hi,

Nash Tsai wrote:
> On Thu, Apr 8, 2010 at 4:18 PM, Martin Storsj? <martin at martin.st> wrote:
>> On Thu, 8 Apr 2010, Luca Abeni wrote:
>>
>>> Martin Storsj? wrote:
>>> [...]
>>>> All current packetizers buffer the data they're going to send within
>>>> RTPMuxContext->buf, but splitting ff_rtp_send_data this way does allow us to
>>>> avoid one layer of buffering, if each individual packetizer wants to. You
>>>> need to hear from Luca A if he's ok with this approach.
>>> Well, I am not sure about this. I am currently undecided, but it other
>>> people like the idea of splitting ff_rtp_send_data(), I am ok with it.
>>> Let's just be sure that there are enough people in favour of this solution.
>> I'm a bit undecided, too. I like the current one for simplicity, but it
>> does indeed require one extra copy of data.
> 
> What I can see here is that it's trying to reuse the implementations
> of writing RTP header and update seq number on what is already inside
> ff_rtp_send_data(), and have better performance without making a copy
> of data, isn't this be better?

As it's done in the current patch, no:
- If there is a performance problem, then it must be fixed for all the
   packetisers. Introducing a new one that does things in a different way
   respect to the others is not a good idea.
- If there is not a performance problem (I do not know, I did not see
   any number), then optimising this part of the code is not useful.

So, I think that first we need to decide if a new approach (splitting
ff_rtp_send_data()) is needed; if it is, then we need a small patch
performing the split, followed by a patch converting the current
packetisers. Then, the new one (implementing support for the MJPEG
payload type) can be committed.


>>> In any case, what I do not want to see is a non-uniformity between the
>>> various packetisers. If we decide to split ff_rtp_send_data(), then all
>>> the packetisers must be converted. Having some packetisers implemented in
>>> the old way and some in the new one is (IMHO) not acceptable.
>> Yes, uniformity is important.
> 
> I tend to agree this, but with sacrifice of performance it's rather
> hard for me to fully agree on it.

Noone wants to sacrifice performance :)
But if there are performance problems for the MJPEG packetiser, then
the same problems are there for all the other packetisers. Fixing the
performance issues only in one case is not a good idea. And people
implementing support for new payload types in the future might use the
"bad" (badly performing) code as a template.

So, if we want to avoid an extra memcpy() let's avoid it for all the
payload types, not only for a random one...


				Luca