[Ffmpeg-devel] [RFC/Patch] native mms code...

Michael Niedermayer michaelni
Wed Mar 7 10:20:50 CET 2007


Hi

On Tue, Mar 06, 2007 at 11:36:49AM -0600, Ryan Martell wrote:
> 
> On Mar 5, 2007, at 8:48 PM, Michael Niedermayer wrote:
> 
> >Hi
> >
> >On Tue, Feb 27, 2007 at 02:28:42PM -0600, Ryan Martell wrote:
> >>On Jan 5, 2007, at 3:56 PM, Ryan Martell wrote:
> >>>On Jan 5, 2007, at 2:23 PM, Michael Niedermayer wrote:
> >>>>On Fri, Jan 05, 2007 at 11:36:15AM -0600, Ryan Martell wrote:
> >>>>[...]
> >[...]
> >
> >>
> >>>>>[...]
> >>>>>>                   if((mms->incoming_flags == 0X08) || (mms-
> >>>>>>>incoming_flags == 0X0C))
> >>>>>>                   {
> >>>>>>                       // we are done with the header stuff.
> >>>>>>                       ByteIOContext byte_context;
> >>>>>>
> >>>>>>                       fprintf(stderr, "Got the full header! (%
> >>>>>>d bytes)\n", mms->asf_header_length);
> >>>>>>                       // parse the header....
> >>>>>>                       init_put_byte(&byte_context, mms-
> >>>>>>>asf_header, mms->asf_header_length, 0, NULL, NULL, NULL, NULL);
> >>>>>>                       if(ff_asf_read_header(mms-
> >>>>>>>av_format_ctx, &mms->asf_context, &byte_context)==0)
> >>>>>>                       {
> >>>>>>                           int stream_count= 2;
> >>>>>>                           int ii;
> >>>>>>
> >>>>>>                           //  send the streams we want back...
> >>>>>>                           put_flush_packet(&mms->packet_data);
> >>>>>>                           put_le32(&mms->packet_data,
> >>>>>>stream_count);
> >>>>>>                           for(ii= 0; ii<stream_count; ii++)
> >>>>>>                           {
> >>>>>>                               put_le16(&mms->packet_data,
> >>>>>>0xffff); // flags
> >>>>>>                               put_le16(&mms->packet_data, ii
> >>>>>>+1); // stream id
> >>>>>>                               /*
> >>>>>>                                00 = stream at full frame rate;
> >>>>>>                                01 = only stream key frames;
> >>>>>>                                02 = no stream, switch it off.
> >>>>>>                                */
> >>>>>>                               put_le16(&mms->packet_data,
> >>>>>>00); // stream at full frame rate
> >>>>>
> >>>>>AVStream.discard could be used here maybe?
> >>>>
> >>>>Not familiar with that; will check it out.
> >>>
> >>>Again, I don't think so (unless I'm missing something).  This is
> >>>telling the server what to send; it has no bearing on the client
> >>>(except, of course, that the client can only play what the server
> >>>sends)
> >
> >my idea IIRC (its some time ago when i suggested AVStream.discard ...)
> >was that the user sets AVStream.discard for all but 1 video and
> >audio stream and the mms code then requests these from the server
> >so the server sends just them ...
> 
> I think that's possible, after some refactoring; I've now added mms  
> through http tunneling to the code.
> 
> To be clear:
> the mms_read_header would be called, and would return streams for  
> everything, and would fill in the bitrates for those streams based on  
> the header parameters.
> 
> then, the application would set AVDiscard on anything that it didn't  
> want, based on stream types, bitrates, etc.
> 
> Finally, on the first call to mms_read_packet, I would negotiate the  
> streams desired with the server, based on the AVDiscard flag.
> 
> Is that what you're thinking?

yes exactly


> 
> The only problem with that is that if you ran it in default mode, on  
> an mms server that had multiple audio streams, I'm not sure what  
> ffplay.c would do.
> 
> >>>
> >>>Will work on implementing the other changes over the weekend and
> >>>getting a another version up for RFC.
> >>
> >>Here'a another version.  There are lots of internal changes, mainly
> >>designed to get it to work as cleanly as possible with asf.c.  This
> >>version isn't tested as much as the previous version (I've only
> >>tested it on live video streams, not on demand stuff), but it does
> >>work for that.
> >>
> >>Let me know what people think....
> >
> >its big, which makes review hard, if you can split this into
> >a series of incremental patches it would simplify and shorten  
> >review cycles
> >alot
> 
> Um... not sure how best to do that; and now it's even larger, as it  
> has the http code in it as well. ;-)

well adding more code (the http code sounds like it is unrelated and could
easily be kept as a seperate incremental change) will lead to rejection
of the patch

not spliting it further will significantly delay its review as its much
harder to review such large patches


[...]
> >[...]
> >>static void generate_guid(char *dst)
> >>{
> >>    AVRandomState random_state;
> >>    char digit[]= "0123456789ABCDEF";
> >>    int ii;
> >>
> >>    av_init_random(1234, &random_state);
> >>    for(ii= 0; ii<36; ii++) {
> >>        switch(ii) {
> >>            case 8:
> >>            case 13:
> >>            case 18:
> >>            case 23:
> >>                *dst++= '-';
> >>                break;
> >>
> >>            default:
> >>                *dst++= digit[av_random(&random_state)%sizeof 
> >>(digit)];
> >>                break;
> >>        }
> >>    }
> >>    *dst= '\0';
> >
> >what good does generating the GUID with av_random but a fixed seed do?
> >you could as well hardcode the GUID
> 
> I know, but I would like that to change.  Maybe I can set the seed  
> based on a hash of the url or something.

and what good would that do ?
id just set the GUID to a fixed number unless there is an actual
problem with this in which case id set it to the IP or a simple
hash of the url or something, no seed no pseudo random numbers needed


[...]
> >[...]
> >>static int mms_read_packet(AVFormatContext *s,
> >>                            AVPacket *pkt)
> >>{
> >>    MMSState *mms = s->priv_data;
> >>    int result= 0;
> >>
> >>//    fprintf(stderr, "mms_read_packet!\n");
> >>    if(mms->state==STREAMING || mms->state==STREAM_PAUSED || mms- 
> >>>state==AWAITING_STREAM_START_PACKET)
> >>    {
> >>        result= asf_demuxer.read_packet(s, pkt);
> >>        if(pkt->pts != AV_NOPTS_VALUE && mms- 
> >>>base_timestamp==AV_NOPTS_VALUE)
> >>        {
> >>            mms->base_timestamp= pkt->pts;
> >>//            fprintf(stderr, "Setting base timestamp to %lld!\n",  
> >>mms->base_timestamp);
> >>        }
> >>
> >>        // required for live to make ffplay show something  
> >>sensible; should be done in asf, but looks more complicated there.
> >>        if(mms->base_timestamp != AV_NOPTS_VALUE)
> >>        {
> >>//            fprintf(stderr, "Adjusting timestamp by %lld!\n",  
> >>mms->base_timestamp);
> >>            if(pkt->pts != AV_NOPTS_VALUE)
> >>            {
> >>//                fprintf(stderr, "Adjusting PTS timestamp from % 
> >>lld to %lld!\n", pkt->pts, pkt->pts-mms->base_timestamp);
> >>                pkt->pts -= mms->base_timestamp;
> >>            }
> >>            if(pkt->dts != AV_NOPTS_VALUE)
> >>            {
> >>//                 fprintf(stderr, "Adjusting DTS timestamp from % 
> >>lld to %lld!\n", pkt->dts, pkt->dts-mms->base_timestamp);
> >>                pkt->dts -= mms->base_timestamp;
> >>            }
> >
> >what is the base_timestamp subtraction good for?
> 
> live streams.  if you don't do this, the stats on ffplay will show  
> really odd times.  By doing this, it shows the amount of time you've  
> been watching the stream....

well, if the only issue is odd stats then id remove it

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

I know you won't believe me, but the highest form of Human Excellence is
to question oneself and others. -- Socrates
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20070307/1bbc300f/attachment.pgp>



More information about the ffmpeg-devel mailing list