[FFmpeg-devel] add MJPEG support into RTP output

[Nash Tsai]

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;

>
> 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