[FFmpeg-devel] [PATCH] ffserver rtsp bug fixes

[Howard Chu]

Luca Abeni wrote:
> On 05/19/2010 09:10 AM, Howard Chu wrote:
> [...]
>>>>> I think this is the wrong solution. The (IMHO) correct one would be to
>>>>> use
>>>>> a bitstream filter to convert the NAL syntax.
>>>>
>>>> Why is that a better solution?
>>>
>>> Because in this way we can separate different functionalities (converting
>>> the bitstream syntax - in the bitstream filter - and packetising the
>>> bitstream - in the rtp mixer) in different parts of the code. And we can
>>> avoid code duplication (and we can reduce the complexity of the rtp muxer
>>> code).
>>> AFAIR, the needed bitstream filter is already there, it just needs to be
>>> used (I used this solution in one of my programs, some time ago... If I
>>> remember well, I just needed to fix some minor issue. But then it
>>> worked).
>>
>> I don't think the muxer's complexity has been harmed by these patches.
>
> They add (probably duplicated) code to rtpenc_h264.c, and to sdp.c.

>> On the other hand, I've just taken a look at the h264_mp4toannexb filter
>> source code and it is quite dense. Also, anything that does
>> "alloc_and_copy" is a bad idea from a memory and CPU resource perspective.

> Well, there can be 2 cases:
> 1) bitstream filters are useless ->  in this case I agree that something like
>      your patch (removing code duplication) can be ok
> 2) bitstream filters are useful and should be used ->  in this case, I believe
>      a bitstream filter should be used, here.
>
> I am not qualified to say if 1) or 2) is the case (maybe someone else can
> comment on this), but the fact that bitstream filters exist seems to suggest
> that 2) is the case...

In particular, h264_mp4toannexb_bsf is not relevant. RTP sends raw NAL units, 
not H.264 Annex B byte streams.

>>>> so that libavcodec/h264.c doesn't have to do exactly these
>>>> same steps (after all, that's where I lifted this code from) ?
>>>
>>> This smells like code duplication... :)
>>
>> Indeed. These functions ought to be collected into a single place. I
>> believe the "clean split" between decoder, muxer, and network layer
>> you're alluding to is a distinct disadvantage here. Anything that
>> touches H264 has to understand how to parse NALUs; that code belongs in
>> a single place usable from all of those layers.
>
> So, a helper function doing this can be introduced, and used in all the
> relevant places...

I've written such a helper function here. However, I haven't found any other 
places where it is useful yet. In particular, factoring out the similar code 
in h264.c decode_nal_units() actually makes that function more complicated and 
less efficient, so it was best to just leave that as-is.

It also doesn't noticably simply sdp.c. I think this is a case where the 
actual amount of code is too small to make re-use worth the cost of restructuring.
-- 
   -- Howard Chu
   CTO, Symas Corp.           http://www.symas.com
   Director, Highland Sun     http://highlandsun.com/hyc/
   Chief Architect, OpenLDAP  http://www.openldap.org/project/
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: avc.txt
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100528/e046edf1/attachment.txt>
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: rtp.txt
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100528/e046edf1/attachment-0001.txt>