[FFmpeg-devel] [PATCH] SHOUTcast HTTP Support

[Micah F. Galizia]

On 10-01-09 09:23 PM, Michael Niedermayer wrote:
> On Sat, Jan 09, 2010 at 08:27:17PM -0500, Micah F. Galizia wrote:
>> On 10-01-09 08:09 PM, Micah F. Galizia wrote:
>>> On 10-01-09 07:15 PM, Micah F. Galizia wrote:
>>>> Greetings,
>>>>
>>>> I've attached a patch adds support for SHOUTcast metadata. This patch
>>>> attempts to connect via HTTP with icy metadata enabled. If the
>>>> connection fails, connection is attempted again without icy metadata (in
>>>> case the server doesn't support it).
>>>>
>>>> As new metadata is received, it is updated in the HTTPContext, and a
>>>> flag is set to indicate that the metadata has been updated. URLProtocol
>>>> has two new methods that indicate that the metadata has changed, and
>>>> pass the new metadata (not a copy, but the actual pointer) back to the
>>>> AVFormatContext through the ByteIOContext. Note, that the definition of
>>>> AVMetadata and AVMetadataConv has been moved from avformat.h to avio.h
>>>> so that they are defined before the definition of URLProtocol.
>>>>
>>>> ByteIOContext has been updated to wrap the new URLProtocol methods so
>>>> that as packets are read the metadata can be returned. If the
>>>> ByteIOContext reports new metadata, then the updated fields are added to
>>>> the AVFormatContext.
>>>>
>>>> A new method has been added to metadata API to allow applications to
>>>> monitor changes in metadata (av_metadata_updated), and ffplay has been
>>>> updated to use it.
>>>>
>>>> One point I would like to note is that I was never able to find a server
>>>> that didn't like the icy-metadata request, so I haven't really verified
>>>> that reconnection without icy-metadata works. If someone knows a server
>>>> that will respond with an error, I would like to know.
>>>
>>> This version is patchecked.
>>
>> Sorry for the spam. After a discussion on IRC, I've removed the unnecessary
>> comments.
>
> Some comments about your implementation
>
> * Simply updating metadata and setting some flag is not ok
>    We have AVChapters that can store metadata that applies to
>    some timespan. The way you do it only the current is available.
>    Of course theres nothing wrong with letting th user know by some
>    means that a new chapter has begun if we lack an easy way to detect
>    this currently

AVChapters are now passed back from the URLProtocol instead of 
AVMetadata. I had a look at some apps that use libavformat (well, just 
MythMusic) and it seems that they keep track of time on there own, so 
they should be capable of finding their own chapter.  AVFormatContext 
also tells them the number of chapters, so if they are expecting new 
chapters (and want to know when they are added), they could keep their 
own chapter counter and then compare it to nb_chapters. To facilitate 
using AVChapters with a URLProtocol, AVChapter's definition was moved to 
avio.h

> * We do need some kind of seperation between protocol and demuxer
>    metadata. That is a user application should be able to extract
>    both a title from protocol level and one from demuxer level if they
>    differ. I belive this wouldnt work with your code
>    The simplest way i see for this would be to prefix all the metadata
>    keys with "Icy:" or something like that
>    (but i need to think more abiut the exact syntax to keep this simple
>    to use)

I never got a good sense of if I should or should not be prefixing, but 
personally, since any SHOUTcast metadata I have seen is prefixed with 
"Stream" (StreamTitle, StreamUrl, etc), I don't think this is really 
necessary.

> * We should keep the API for the user application simple
>    I think your code achives that though but we must make sure that
>    this stays simple ...

It is essentially the same implementation with some throw-away code to 
test the chapters in ffplay, so hopefully it is still simple enough. 
Also, now there are no new methods in avformat.h.

I would also like some feedback on my initialization values in utils.c. 
I assumed that since these radio streams don't broadcast how long they 
will be streaming, setting the duration to INT64_MAX was appropriate 
since they will broadcast longer than most users will listen. Also, I 
set the stream start time to the first chapters start time.

Thanks in advance!
-- 
Micah F. Galizia
micahgalizia at gmail.com

"The mark of an immature man is that he wants to die nobly for a cause, 
while the mark of the mature man is that he wants to live humbly for 
one."   --W. Stekel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: shoutcast4.patch
Type: text/x-diff
Size: 13460 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100113/8e5d0f4a/attachment.patch>