[FFmpeg-devel] [PATCH] SHOUTcast HTTP Support

[Michael Niedermayer]

On Thu, Apr 08, 2010 at 09:14:53PM -0400, Micah F. Galizia wrote:
> 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.

what about a array of pointers to AVFormatContext in AVFormatContext?

[...]
>>
>>> 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.

i would not bet on mp3 & vorbis sizes being sumable like this.
The correct way id guess would be to split/demux the "stream" and provide
the child demuxer just with its own data


>
> [...]
>
> 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.

try some dts variables from AVFormatContext.streams[0] maybe cur_dts
note this is hackish not clean but i dont know a clean variant atm

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Many things microsoft did are stupid, but not doing something just because
microsoft did it is even more stupid. If everything ms did were stupid they
would be bankrupt already.
-------------- 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/20100409/3f121d7e/attachment.pgp>