[FFmpeg-devel] add MJPEG support into RTP output

[Nash Tsai]

Hi Martin,

On Thu, Apr 8, 2010 at 10:05 PM, Nash Tsai <nash.tsai at gmail.com> wrote:
> Hi Martin,
>
> On Thu, Apr 8, 2010 at 8:57 PM, Martin Storsj? <martin at martin.st> wrote:
>> On Thu, 8 Apr 2010, shawn shawn wrote:
>>
>>> ? ?According to all said, i have change it,
>>> ?1. use ff_rtp_send_data, not split it
>>> ?2. change a=rtpmap:%d MJPEG/30000\r\n to a=rtpmap:%d JPEG/90000\r\n
>>
>> Since this should use the static payload type 26 (right?), this kind of
>> shouldn't be needed at all.
>>
>>> ?3. tidy up some code and add some comment about rfc number
>>> ?4. add a new file rtpenc_mjpeg.c
>>
>> Ok, now it's starting to look a bit better. You still haven't addressed my
>> comment about using a struct for creating the header may have
>> endian-issues with the bitfields, and may have other issues with
>> compiler-dependent struct packing.
>>
>> You may have taken inspiration to the struct from the RFC, but note this
>> comment in the RFC just above the example code:
>>
>> ? For clarity and brevity, the structure definitions are only valid for
>> ? 32-bit big-endian (most significant octet first) architectures. Bit
>> ? fields are assumed to be packed tightly in big-endian bit order, with
>> ? no additional padding. Modifications would be required to construct a
>> ? portable implementation.
>
> For what I looked it's handling endian issue pretty well, as only the
> struct member greater than 1 octet is affected, and that is the member
> JPEGHrd::off, and the handling code are as following:
>
> jpghdr.off = htonl(offset) >> 8;

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:

if (htonl(offset) != offset)
   jpghdr.off = htonl(offset) >> 8; /* we only interested in least
significant 3 octets here */
else
   jpghdr.off = offset;

>
>>
>> So instead of trying to mimic the data format with a struct, whose layout
>> you can't control totally to the level needed by a communications
>> protocol, you'll have to manually build the corresponding header in an
>> uint8_t buffer, using normal bit operators and e.g. AV_WB* macros.
>>
>>> ?5. ?"tspec", "type", and "q" fields always set to 0, because rfc
>>> defined a range, maybe we can set some input parameters, but now i
>>> have not do it.
>>
>> After browsing the RFC a briefly, I'm not sure that leaving all these as
>> zero is ok.
>>
>>> ?I have test VLC media player, add this header it not work, if don't
>>> add it can work, However, according to rfc documents 2435,
>>> i think we should add it.
>>
>> I don't understand what you're trying to say here. In which configuration
>> does it work, and in which does it not work?
>>
>> // Martin
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel at mplayerhq.hu
>> https://lists.mplayerhq.hu/mailman/listinfo/ffmpeg-devel
>>
>
> Nash Tsai
>