[FFmpeg-devel] [PATCH] RTP H.264 / AVC support

[Michael Niedermayer]

On Wed, Jun 02, 2010 at 03:45:18PM -0700, Howard Chu wrote:
> Luca Abeni wrote:
>> Ok, I re-read the whole thread, and I agree that maybe I have been
>> not clear enough in describing how I think the patch should look like.
>> Let me apologise about that.
>>
>> So, instead of writing another long email to explain my idea (with the
>> risk of being unclear again), I tried to write down some code to
>> better explain it (warning: the attached thing works for me, but it is
>> just a 5 minutes hack intended to explain an idea... It probably misses
>> some error checking, I did not run patchcheck on it, etc...).
>>
>> The attached patch tries to explain how I think the sdp.c modifications
>> should look like (it does not address the rtpenc_h264.c modifications
>> yet). It uses a bitstream filter (and it requires a small modification /
>> improvement to the mp4toannexb bsf), because the SDP is written only one
>> time, before starting the stream... So a small memcopy and memory
>> reallocation cannot be a problem.
>> I ran some minimal tests, and it works here (maybe it fails in some
>> other cases, but it is just a proof of concept)
>
> OK, I've taken your patch and moved a few things around and run it thru 
> patcheck. The only warning from it now is spurious (possibly unused var 
> which is definitely used).
>
> And I've brought my other patches forward since you seemed to be OK with 
> them in your previous reply. So again, it's working for me too.
>>
>> Regarding the rtpenc_h264.c modifications, I think that introducing a
>> static inline const uint8_t *ff_avc_find_startcode_avc(const uint8_t *p, 
>> const uint8_t *end, int nal_length_size)
>> or a
>> static inline int ff_avc_nal_size_avc(const uint8_t *p, int 
>> nal_length_size)
>> helper (that can be shared with the bsf and with h264.c) can help in
>> writing a small patch.
>
> I took a stab at this but just getting the length isn't really very useful 
> in terms of minimizing code.
>
> #include "libavcodec/bytestream.h"
> static inline int ff_avc_nal_size_avc(const uint8_t **p, int 
> nal_length_size)
> {
>     int nalsize;
>     switch(nal_length_size) {
>     case 1: nalsize = **p; break;
>     case 2: nalsize = AV_RB16(*p); break;
>     default: nalsize = AV_RB32(*p); break;
>     }
>     *p += nal_length_size;
>     return nalsize;
> }

while(nal_length_size--)
    nalsize= 256*nalsize + *(*p)++;

also i will reject duplication of the nal unit parsing code.
ive seen a bug or 2 in the related code in h264.c the maintaince
burden is not reasonable if this code is duplicated further

and non trivial functions should be documented

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I have often repented speaking, but never of holding my tongue.
-- Xenocrates
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100603/a5fc5f6d/attachment.pgp>