[FFmpeg-devel] [PATCH] MMS protocol support patch 1
Michael Niedermayer
michaelni
Sat Oct 13 02:37:04 CEST 2007
On Thu, Oct 11, 2007 at 02:59:00PM +0200, Bj?rn Axelsson wrote:
> On Wed, 2007-10-10 at 04:09 +0200, Michael Niedermayer wrote:
> > Hi
> >
> > On Tue, Oct 09, 2007 at 02:00:25PM +0200, Bj?rn Axelsson wrote:
> > [...]
> > > I have also fixed the issues mentioned by Diego earlier in this thread
> > > (some trailing whitespace and many long lines).
> > >
> > > Attached patches:
> > > 1. aviobuf_resetbuf.diff: add functionality to set the direction of a
> > > ByteIOContext buffer.
> > > 2. mms_protocol_base.diff: basic mmsh protocol support
> > > 3. mms_seek_hack.diff: Add MMS seek and pause support to the ASF
> > > demuxer.
> > >
> > > Thanks to Neil Brown for suggesting using quilt for patch management,
> > > and to Michael for taking his time for the review.
> > [...]
> >
> > (1. aviobuf_resetbuf.diff)
> >
> > patch ok
>
> Yay!
(many) small patches reach svn quicker in general than big ones ...
-> spliting is always a good idea ... (just see the PAFF patch)
[...]
> Index: libavformat/asf.c
> ===================================================================
> --- libavformat/asf.c.orig 2007-10-09 10:38:02.000000000 +0200
> +++ libavformat/asf.c 2007-10-11 14:43:32.403185962 +0200
> @@ -24,6 +24,8 @@
> #include "asf.h"
> #include "common.h"
>
> +extern void ff_mms_set_stream_selection(URLContext *h, AVFormatContext *format);
> +
> #undef NDEBUG
> #include <assert.h>
>
> @@ -105,6 +107,16 @@
> }
> #endif
>
> +#ifdef CONFIG_MMSH_PROTOCOL
> +static int is_mms(ByteIOContext *pb)
> +{
> + return url_fileno(pb) && url_fileno(pb)->prot &&
> + !strcmp(url_fileno(pb)->prot->name, "mmsh");
> +}
> +#else
> +#define is_mms(x) 0
> +#endif
> +
> static void get_str16_nolen(ByteIOContext *pb, int len, char *buf, int buf_size)
> {
> char* q = buf;
> @@ -532,6 +544,10 @@
> }
> }
>
> + /* Give info about ourselves to the mms protocol */
> + if(is_mms(pb))
> + ff_mms_set_stream_selection(url_fileno(pb), s);
> +
> return 0;
>
> fail:
the above is ok (if it where a seperate patch in a seperate email :)
[...]
> +MMSProtocol mmsh_mmsprotocol = {
ff prefix missing
[...]
> +static int mms_close(URLContext *h);
> +int ff_mms_play(URLContext *h);
> +int ff_mms_seek(URLContext *h, int stream_index, int64_t timestamp, int flags);
why not order the functions so that these arent needed?
[...]
> + char *src = path;
> + while(*src) {
> + seed += *src;
> + src++;
> + }
for(src= path; *src; src++)
seed += *src;
[...]
> + if(packet_type == SC_PACKET_ASF_MEDIA_TYPE ||
> + packet_type == SC_PACKET_ASF_HEADER_TYPE)
this can be vertically aligned
> + continue;
> +
> + if(mms->state == STREAM_PAUSED) {
> + // TODO: result = AVERROR(EAGAIN);
> + break;
> + } else if(mms->state == STREAM_DONE || mms->state == STATE_ERROR) {
> + break;
> + } else if(mms->state == AWAITING_ASF_HEADER) {
> + /* we have reset the header; spin though the loop. */
> + while(mms->state != STREAMING && mms->state != STATE_ERROR) {
> + ff_mms_packet_state_machine(mms);
> + }
can this end in an infinite loop?
[...]
> + MMSContext *mms;
> +
> + mms = av_malloc(sizeof(MMSContext));
> + if (!mms)
> + return AVERROR(ENOMEM);
> + memset(mms, 0, sizeof(MMSContext));
MMSContext *mms= av_mallocz()
> + h->priv_data = mms;
> + h->is_streamed = 1; /* Note: seeking is possible, but not preferred */
maybe we should extend is_streamed to more than just 0/1 so the possible but
slow could be clearly indicated (this of course has nothing to do with this
patch ...)
> + mms->av_class = &mmscontext_class;
> +
> + url_split(protocol, sizeof(protocol), authorization, sizeof(authorization),
> + mms->host, sizeof(mms->host), &mms->port,
> + mms->path, sizeof(mms->path), uri);
> +
> + if(strcmp(protocol, "mmsh") == 0 || strcmp(protocol, "mms") == 0) {
> + mms->protocol = &mmsh_mmsprotocol;
> + } else {
> + goto fail;
> + }
how can this else be reached? if it cant then this should be a assert()
[...]
> +/** Close the MMSH/MMST connection */
> +static int mms_close(URLContext *h)
> +{
> + MMSContext *mms = (MMSContext *)h->priv_data;
> +
> + if(mms->mms_hd) {
> + /* send the close packet if we should. */
> + if(mms->state != STATE_ERROR && mms->protocol->send_shutdown_message)
> + mms->protocol->send_shutdown_message(mms);
> +
> + close_connection(mms);
> + }
> +
> + if(mms->asf_header)
> + av_free(mms->asf_header);
the if() is unneeded and av_freep() seems a better choice as well
[...]
> + switch(whence) {
> + case SEEK_END:
> + case AVSEEK_SIZE:
> + /* file_size is probably not valid after stream selection,
> + * so we just disable this */
> + return -1;
> + case SEEK_CUR:
> + cur_pos = mms->incoming_packet_seq * mms->asf_context.packet_size +
> + mms->asf_header_read_pos +
> + (mms->media_packet_read_ptr - mms->media_packet_incoming_buffer);
> +
> + /* special case for no-op. returns current position in bytes */
> + if(offset == 0)
> + return cur_pos;
> +
> + offset += cur_pos;
> +
> + break;
> + case SEEK_SET:
> + break;
> +
> + default:
> + return -1;
> + }
all the return -1 cases could be placed together
[...]
> +/** Like AVInputFormat::read_pause().
> + * @see AVInputFormat::read_pause()
> + */
> +int ff_mms_pause(URLContext *h)
> +{
> + MMSContext *mms = h->priv_data;
> +
> + if(mms->state == STREAMING) { // || mms->state == STREAM_DONE) {
> + mms->protocol->pause(mms);
> +
> + /* wait for acknowledge (throwing away any incoming media) */
> + while(mms->state != STREAM_PAUSED && mms->state != STATE_ERROR)
> + ff_mms_packet_state_machine(mms);
cant this end in an infinite loop?
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
It is dangerous to be right in matters on which the established authorities
are wrong. -- Voltaire
-------------- 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/20071013/8dc2b71a/attachment.pgp>
More information about the ffmpeg-devel
mailing list