[FFmpeg-devel] add MJPEG support into RTP output

[Nash Tsai]

Hi Martin,

On Thu, Apr 8, 2010 at 11:35 PM, Martin Storsj? <martin at martin.st> wrote:
> On Thu, 8 Apr 2010, Nash Tsai wrote:
>
>> I was wrong, the code was only handling little endian case, but the
>> JPEGHdr struct doesn't need to worry about endian-ness as the handling
>> of the network byte order should be taken before sending it off to the
>> network, I may write it as following:
>
> The field placement actually seems to be constant across some
> architectures in this case, but that cannot be relied upon. If you
> would have had bitfields with a size less than a byte, they would have
> been laid out differently on different architectures.
>
>> 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.

>
> That's simple, clear and doesn't have any lurking portability issues.
>
> // Martin
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at mplayerhq.hu
> https://lists.mplayerhq.hu/mailman/listinfo/ffmpeg-devel
>

Nash Tsai