[FFmpeg-devel] [PATCH] SHOUTcast HTTP Support

[Micah F. Galizia]

On 10-04-06 09:40 PM, Michael Niedermayer wrote:
> On Tue, Apr 06, 2010 at 09:06:35PM -0400, Micah F. Galizia wrote:
>> On 10-04-06 06:37 PM, Michael Niedermayer wrote:
>>> On Wed, Mar 31, 2010 at 06:43:10PM -0400, Micah F. Galizia wrote:
>>>
>>
>> [...]
>>
>>>> +    /* there is no shoutcast in shoutcast */
>>>> +    if (!strcmp(sc->child->name, SHOUTCAST_DEMUXER_NAME)) {
>>>> +        av_log(s, AV_LOG_ERROR, "ERROR: Child demuxer is %s\n",
>>>> +               sc->child->name);
>>>> +        return AVERROR_IO;
>>>> +    }
>>>
>>> thats fragile, we just need a second shoutcast like demxuer and
>>> each with such check would not be enough
>>
>> I have observed the s->iformat and sc->child pointers values are the same
>> with when both are shoutcast demuxers.  Can I rely on that, or did you have
>> something else in mind?
>
> shoutcast(foobar(shoutcast(foobar(...))))
>
> checking the immedeate first child is not enough
> it depends on the non existence of a second shoutcast like demuxer.
> thats fragile as we would not consider that when we add such a demuxer
>

Perhaps we can add a new function pointer to AVInputFormat.  Something 
like AVInputFormat (*get_child)(char *name);. Any demuxer that has 
children can check the child name or recurse down until said demuxer is 
found or NULL is returned.

>>
>>>
>>>> +
>>>> +    if (!(sc->child_priv = av_mallocz(sc->child->priv_data_size))) {
>>>> +        av_log(s, AV_LOG_ERROR,
>>>> +               "Unable to allocate SHOUTcast child private data.\n");
>>>> +        return AVERROR_NOMEM;
>>>> +    }
>>>> +
>>>> +    /* read past the HTTP header (after probing the stream is reset) */
>>>> +    url_fseek(s->pb, offset, SEEK_CUR);
>>>> +
>>>
>>>> +    /* ensure that we set the data position ahead if the child demuxer
>>>> moves
>>>> +     * the buffer pointer forward. TODO Use pb->pos if possible. */
>>>> +    offset = (unsigned long)(s->pb->buf_ptr);
>>>
>>> whatever this is it has undefined behavior
>>>
>>
>> What I need is to know how the child demuxer has manipulated the buffer
>> (did it move the pointer forwards or backwards?). I need to know this
>
> url_ftell() ?

ty!

>
>> because it affects the  index within the actual data between metadata
>> chunks. If this is unacceptable and there isn't a preferred way getting
>> this information, I can try to seek back to the beginning of the stream and
>> failing that close, reopen, and seek beyond the http header to the start of
>> real data.
>>
>>>> +    s->priv_data = sc->child_priv;
>>>> +    ret = sc->child->read_header(s, ap);
>>>> +    s->priv_data = sc;
>>>> +    sc->datapos = (unsigned long)(s->pb->buf_ptr) - offset;
>>>> +
>>>> +    return ret;
>>>> +}
>>>> +
>>>
>>>> +static int shoutcast_read_packet(struct AVFormatContext *s, AVPacket
>>>> *pkt)
>>>> +{
>>>> +    int ret;
>>>> +    ByteIOContext *pb = s->pb;
>>>> +    SHOUTcast *sc = s->priv_data;
>>>> +
>>>> +    /* allow the child demuxer to read a packet. */
>>>> +    s->priv_data = sc->child_priv;
>>>> +    if ((ret = sc->child->read_packet(s, pkt))<   0) {
>>>> +        return ret;
>>>> +    }
>>>> +    s->priv_data = sc;
>>>
>>>> +    sc->datapos += pkt->size;
>>>
>>> what is datapos supposed to be?
>>> this just looks wrong, it surely is not the "file" position
>>
>> No, it is a counter of how much non-metadata data has been read.  When it
>> is greater than our metadata interval, we must strip the icy metadata out
>> before passing more non-metadata to the child demuxer.
>
> still sounds wrong, the sum or packet sizes is not the same as the num
> of bytes the child demuer consumed. These might match for mp3 but will
> not match for most other cases

The scast demuxer _needs_ to keep track of how far into the stream data 
has been read. And an absolute position wont work either (because the 
interleaved meatdata is variable sized) -- if the child reads past a 
segment of metadata, then the scast demuxer counts are totally invalid! 
Also, if the child demuxer is just consuming stream data without 
returning some sort of count that also wont work.

Is there a correct way to work around this problem? If not, can we 
restrict children to formats that do work? So far I have verified that 
vorbis and mp3 streams play nice.

[...]

One other question I have (the reference comment was cut in my previous 
email) is about chapter times. I understand that the start should be 
based purely on the data we have read up to the point where the metadata 
segment occurs. But I don't have any idea how this should done 
correctly, so a nudge in the right direction would be appreciated.

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