[FFmpeg-devel] add MJPEG support into RTP output

[Nash Tsai]

Hi,

On Fri, Apr 9, 2010 at 1:28 PM, Martin Storsj? <martin at martin.st> wrote:
> On Fri, 9 Apr 2010, Nash Tsai wrote:
>
>> On Thu, Apr 8, 2010 at 11:35 PM, Martin Storsj? <martin at martin.st> wrote:
>> > On Thu, 8 Apr 2010, Nash Tsai wrote:
>> >
>> >> if (htonl(offset) != offset)
>> >> ? ?jpghdr.off = htonl(offset) >> 8; /* we only interested in least
>> >> significant 3 octets here */
>> >> else
>> >> ? ?jpghdr.off = offset;
>> >
>> > This is just horribly obfuscated. As far as I'm concerned, any patch doing
>> > things this way is rejected.
>> >
>> > Just declare a uint8_t hdr[8] and write each field manually into the
>> > correct place of the buffer. For writing the offset you can use e.g.:
>> >
>> > AV_WB24(&hdr[1], offset);
>>
>> Wasn't aware the existence of AV_WB24, thanks, then I would write as
>> following version as simply update the packet buffer after jpghdr has
>> written into it:
>>
>> + ? ? ? ?memcpy(q, &jpghdr, sizeof(jpghdr));
>> + ? ? ? ?AV_WB24(&q[1], jpghdr.off);
>> + ? ? ? ?memcpy(q + sizeof(jpghdr), buf1, len);
>> + ? ? ? ?ff_rtp_send_data(s1, q, len + sizeof(jpghdr), (len == size));
>> +
>> + ? ? ? ?buf1 += len;
>> + ? ? ? ?size -= len;
>> + ? ? ? ?jpghdr.off += len;
>>
>> So I still get to maintain the details of jpghdr with precise struct
>> member naming and values.
>
> No, you need to get rid of the jpghdr struct completely. You cannot assume
> that its size and field layout will be consistent across architectures,
> and it is all too easy to make more endianness mistakes like the one with
> the offset. People compiling with other compiler flags may also get a
> struct with a different layout not matching the one in the RFC.

Yap, you are quite right about the struct alignment may varies, so
should go with your suggestion, seems easier, or using "#pragma
options compact".

>
> // Martin
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at mplayerhq.hu
> https://lists.mplayerhq.hu/mailman/listinfo/ffmpeg-devel
>

Nash Tsai