[FFmpeg-devel] [PATCH] SHOUTcast HTTP Support

[Michael Niedermayer]

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

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

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

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I hate to see young programmers poisoned by the kind of thinking
Ulrich Drepper puts forward since it is simply too narrow -- Roman Shaposhnik
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100110/30bbe1c4/attachment.pgp>