[FFmpeg-soc] [PATCH] decouple mpeg4/aac from rtsp

[Josh Allmann]

Hi,

On 23 June 2010 09:02, Martin Storsjö <martin at martin.st> wrote:
> Hi Josh,
>
> On Tue, 22 Jun 2010, Josh Allmann wrote:
>
>> > 004 breaks receiving video (mpeg4 AND h264) if streamed in conjunction
>> > with AAC, but video seems to work OK in isolation. Which is weird
>> > because 004 should only touch the AAC codepath.
>> >
>> > Been staring at this bug all day, so I'm just gonna turn in what I
>> > have, get some sleep, and hopefully dream up a fix.
>
> Whoa, this is good stuff. Can't wait to commit it :-)
>
> I found the missing thing that messes up streams with both audio and video
> - in the code branch in rtpdec.c for formats without a parse_packet
> function, the generic code sets pkt->stream_index = st->index, which you
> need to do in your dynamic payload handler.
>

Yep, thanks for the second pair of eyes...

> Except for that, patch #5 had some issues where you had forgotten to
> rename infos to data - I guess you've missed to commit something here?
>

Yeah, sorry for the broken commits. Fixed, but I tried to keep the
diffs as small as possible, so I kept infos (the original name) did
another patch (007) that changes infos to data, to maintain
consistency with the rest of rtpdec*.

> In the same patch, you've also got a case where you do if (ptr)
> av_free(ptr), which is't necessary, av_free() can handle that itself.
>

Fixed

> Also, you don't need any new_context/free_context for mp4v_es, since it
> doesn't use that context. In order to do that, you need to make sure that
> you won't accidentally use the PayloadContext in parse_sdp_line unless the
> codec is AAC.
>

Fixed

> In free_context, you should move the opening brace of the for loop. :-)
>

Fixed

> I'm not sure about the copyright owner of this file, that you've set to
> "the FFmpeg developers" - is it possible to dig through the version
> history to see who wrote this initially?
>

Fixed --
Fabrice Bellard added in MPEG-4 video support (r1223), and Romain
Degez did AAC (r4306). I set the copyright to 2010, though.

> Apart from these details - I really like this, good work so far!
>

Awesome -- patch 002 in this series also replaces memchr with strspn,
as noted by Michael and Ronald.

Josh
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Move-skip_spaces-to-libavformat-internal.h.patch
Type: text/x-patch
Size: 4895 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-soc/attachments/20100623/ba4f5240/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-Use-strspn-rather-than-memchr-in-ff_skip_spaces.patch
Type: text/x-patch
Size: 1206 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-soc/attachments/20100623/ba4f5240/attachment-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0003-Decouple-MPEG-4-and-AAC-specific-parts-from-rtsp.c.patch
Type: text/x-patch
Size: 10613 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-soc/attachments/20100623/ba4f5240/attachment-0002.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0004-Moved-more-SDP-FMTP-stuff-from-rtsp.c-to-mpeg4.c.patch
Type: text/x-patch
Size: 7247 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-soc/attachments/20100623/ba4f5240/attachment-0003.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0005-Move-AAC-depacketization-code-in-rtpdec-to-a-proper-.patch
Type: text/x-patch
Size: 7812 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-soc/attachments/20100623/ba4f5240/attachment-0004.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0006-Move-RTPPayloadData-into-rtpdec_mpeg4-and-remove-all.patch
Type: text/x-patch
Size: 10741 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-soc/attachments/20100623/ba4f5240/attachment-0005.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0007-Rename-PayloadContext-to-be-consistently-data.patch
Type: text/x-patch
Size: 5509 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-soc/attachments/20100623/ba4f5240/attachment-0006.bin>