Author: diego Date: Sat Mar 24 17:28:55 2012 New Revision: 34824 Log: stream_ffmpeg: convert to avio API, fix build against shared libavformat patch by Alexis Ballier, alexis.ballier gmail com Modified: trunk/stream/stream_ffmpeg.c Modified: trunk/stream/stream_ffmpeg.c ============================================================================== --- trunk/stream/stream_ffmpeg.c Thu Mar 22 22:04:32 2012 (r34823) +++ trunk/stream/stream_ffmpeg.c Sat Mar 24 17:28:55 2012 (r34824) @@ -26,33 +26,22 @@ #include "m_struct.h" #include "av_helpers.h" -#ifndef URL_RDONLY -#include "libavformat/url.h" -#define url_read_complete ffurl_read_complete -#define url_write ffurl_write -#define url_seek ffurl_seek -#define url_filesize ffurl_size -#define url_close ffurl_close -#define url_open(c, n, f) ffurl_open(c, n, f, NULL, NULL) -#endif - static int fill_buffer(stream_t *s, char *buffer, int max_len) { - int r = url_read_complete(s->priv, buffer, max_len); + int r = avio_read(s->priv, buffer, max_len); return (r <= 0) ? -1 : r; } static int write_buffer(stream_t *s, char *buffer, int len) { - /* url_write retries internally on short writes and EAGAIN */ - int r = url_write(s->priv, buffer, len); - return (r <= 0) ? -1 : r; + avio_write(s->priv, buffer, len); + return len; } static int seek(stream_t *s, off_t newpos) { s->pos = newpos; - if (url_seek(s->priv, s->pos, SEEK_SET) < 0) { + if (avio_seek(s->priv, s->pos, SEEK_SET) < 0) { s->eof = 1; return 0; } @@ -61,12 +50,12 @@ static int seek(stream_t *s, off_t newpo static int control(stream_t *s, int cmd, void *arg) { - URLContext *ctx = s->priv; + AVIOContext *ctx = s->priv; int64_t size, ts; double pts; switch(cmd) { case STREAM_CTRL_GET_SIZE: - size = url_filesize(s->priv); + size = avio_size(s->priv); if(size >= 0) { *(off_t *)arg = size; return 1; @@ -75,13 +64,9 @@ static int control(stream_t *s, int cmd, case STREAM_CTRL_SEEK_TO_TIME: pts = *(double *)arg; ts = pts * AV_TIME_BASE; -#ifdef URL_RDONLY - ts = av_url_read_seek(s->priv, -1, ts, 0); -#else - if (!ctx->prot->url_read_seek) + if (!ctx->read_seek) break; - ts = ctx->prot->url_read_seek(s->priv, -1, ts, 0); -#endif + ts = ctx->read_seek(s->priv, -1, ts, 0); if (ts >= 0) return 1; break; @@ -91,7 +76,7 @@ static int control(stream_t *s, int cmd, static void close_f(stream_t *stream) { - url_close(stream->priv); + avio_close(stream->priv); } static const char prefix[] = "ffmpeg://"; @@ -100,7 +85,7 @@ static int open_f(stream_t *stream, int { int flags = 0; const char *filename; - URLContext *ctx = NULL; + AVIOContext *ctx = NULL; int res = STREAM_ERROR; int64_t size; int dummy; @@ -127,16 +112,16 @@ static int open_f(stream_t *stream, int dummy = !strncmp(filename, "rtsp:", 5); mp_msg(MSGT_OPEN, MSGL_V, "[ffmpeg] Opening %s\n", filename); - if (!dummy && url_open(&ctx, filename, flags) < 0) + if (!dummy && avio_open(&ctx, filename, flags) < 0) goto out; stream->priv = ctx; - size = dummy ? 0 : url_filesize(ctx); + size = dummy ? 0 : avio_size(ctx); if (size >= 0) stream->end_pos = size; stream->type = STREAMTYPE_FILE; stream->seek = seek; - if (dummy || ctx->is_streamed) { + if (dummy || !ctx->seekable) { stream->type = STREAMTYPE_STREAM; stream->seek = NULL; }
On Sat, Mar 24, 2012 at 05:28:55PM +0100, diego wrote:
Author: diego Date: Sat Mar 24 17:28:55 2012 New Revision: 34824
Log: stream_ffmpeg: convert to avio API, fix build against shared libavformat
I'd appreciate it if you would not apply changes you have no clue what they do. This changes introduces an additional buffering layer which can cause performance and maybe even other issues.
- /* url_write retries internally on short writes and EAGAIN */ - int r = url_write(s->priv, buffer, len); - return (r <= 0) ? -1 : r; + avio_write(s->priv, buffer, len); + return len;
For writing it even loses the ability to report errors completely, and it is impossible to know when the data will actually be written (since it will only be written when the buffer is full, which might be far too late). So please revert until there is a proper fix for this code. Options are disabling the feature when the url_* code is not available or using some define magic to switch to the avio API when necessary and print a fat warning about the issues.
And sorry for the grumpiness, but I've mentioned the issues with the new official API several times already and for FFmpeg all the necessary url_ symbols are exported again (though the header needs to be copied manually still). So I am annoyed about it being applied with only about a day's notice. On Sat, Mar 24, 2012 at 08:03:35PM +0100, Reimar Döffinger wrote:
On Sat, Mar 24, 2012 at 05:28:55PM +0100, diego wrote:
Author: diego Date: Sat Mar 24 17:28:55 2012 New Revision: 34824
Log: stream_ffmpeg: convert to avio API, fix build against shared libavformat
I'd appreciate it if you would not apply changes you have no clue what they do. This changes introduces an additional buffering layer which can cause performance and maybe even other issues.
- /* url_write retries internally on short writes and EAGAIN */ - int r = url_write(s->priv, buffer, len); - return (r <= 0) ? -1 : r; + avio_write(s->priv, buffer, len); + return len;
For writing it even loses the ability to report errors completely, and it is impossible to know when the data will actually be written (since it will only be written when the buffer is full, which might be far too late). So please revert until there is a proper fix for this code. Options are disabling the feature when the url_* code is not available or using some define magic to switch to the avio API when necessary and print a fat warning about the issues.
On Sat, Mar 24, 2012 at 08:59:34PM +0100, Reimar Döffinger wrote:
And sorry for the grumpiness, but I've mentioned the issues with the new official API several times already and for FFmpeg all the necessary url_ symbols are exported again (though the header needs to be copied manually still). So I am annoyed about it being applied with only about a day's notice.
No worries, I will revert it shortly, was busy the past few days. Diego
On Tue, Mar 27, 2012 at 07:39:39PM +0200, Diego Biurrun wrote:
On Sat, Mar 24, 2012 at 08:59:34PM +0100, Reimar Döffinger wrote:
And sorry for the grumpiness, but I've mentioned the issues with the new official API several times already and for FFmpeg all the necessary url_ symbols are exported again (though the header needs to be copied manually still). So I am annoyed about it being applied with only about a day's notice.
No worries, I will revert it shortly, was busy the past few days.
I've been working on a flag on the FFmpeg side that should disable the buffering and might make this acceptable. Problem is I need to come up with a way to test it properly. The patch required on MPlayer side would be the below, though the write function still needs to be fixed (needs to return error, explicitly call flush for FFmpeg versions that don't have the flag)... ndex: stream/stream_ffmpeg.c =================================================================== --- stream/stream_ffmpeg.c (revision 34832) +++ stream/stream_ffmpeg.c (working copy) @@ -101,6 +101,13 @@ goto out; } +#ifdef AVIO_FLAG_DIRECT + flags |= AVIO_FLAG_DIRECT; +#else + mp_msg(MSGT_OPEN, MSGL_WARN, "[ffmpeg] No support for AVIO_FLAG_DIRECT, might cause performance and other issues." + "Please update to and rebuild against a supported FFmpeg version.\n"); +#endif + if (stream->url) filename = stream->url; else {
On Tue, Mar 27, 2012 at 09:55:57PM +0200, Reimar Döffinger wrote:
On Tue, Mar 27, 2012 at 07:39:39PM +0200, Diego Biurrun wrote:
On Sat, Mar 24, 2012 at 08:59:34PM +0100, Reimar Döffinger wrote:
And sorry for the grumpiness, but I've mentioned the issues with the new official API several times already and for FFmpeg all the necessary url_ symbols are exported again (though the header needs to be copied manually still). So I am annoyed about it being applied with only about a day's notice.
No worries, I will revert it shortly, was busy the past few days.
I've been working on a flag on the FFmpeg side that should disable the buffering and might make this acceptable. Problem is I need to come up with a way to test it properly. The patch required on MPlayer side would be the below, though the write function still needs to be fixed (needs to return error, explicitly call flush for FFmpeg versions that don't have the flag)...
Revert or not then? Diego
On Wed, Apr 04, 2012 at 04:48:39PM +0200, Diego Biurrun wrote:
On Tue, Mar 27, 2012 at 09:55:57PM +0200, Reimar Döffinger wrote:
On Tue, Mar 27, 2012 at 07:39:39PM +0200, Diego Biurrun wrote:
On Sat, Mar 24, 2012 at 08:59:34PM +0100, Reimar Döffinger wrote:
And sorry for the grumpiness, but I've mentioned the issues with the new official API several times already and for FFmpeg all the necessary url_ symbols are exported again (though the header needs to be copied manually still). So I am annoyed about it being applied with only about a day's notice.
No worries, I will revert it shortly, was busy the past few days.
I've been working on a flag on the FFmpeg side that should disable the buffering and might make this acceptable. Problem is I need to come up with a way to test it properly. The patch required on MPlayer side would be the below, though the write function still needs to be fixed (needs to return error, explicitly call flush for FFmpeg versions that don't have the flag)...
Revert or not then?
I'd say it's ok to leave it be as-is for a little bit longer.
On Wed, Apr 04, 2012 at 08:22:54PM +0200, Reimar Döffinger wrote:
On Wed, Apr 04, 2012 at 04:48:39PM +0200, Diego Biurrun wrote:
On Tue, Mar 27, 2012 at 09:55:57PM +0200, Reimar Döffinger wrote:
On Tue, Mar 27, 2012 at 07:39:39PM +0200, Diego Biurrun wrote:
On Sat, Mar 24, 2012 at 08:59:34PM +0100, Reimar Döffinger wrote:
And sorry for the grumpiness, but I've mentioned the issues with the new official API several times already and for FFmpeg all the necessary url_ symbols are exported again (though the header needs to be copied manually still). So I am annoyed about it being applied with only about a day's notice.
No worries, I will revert it shortly, was busy the past few days.
I've been working on a flag on the FFmpeg side that should disable the buffering and might make this acceptable. Problem is I need to come up with a way to test it properly. The patch required on MPlayer side would be the below, though the write function still needs to be fixed (needs to return error, explicitly call flush for FFmpeg versions that don't have the flag)...
Revert or not then?
I'd say it's ok to leave it be as-is for a little bit longer.
I _think_ it should be fixed now (together with latest FFmpeg).
participants (3)
-
diego -
Diego Biurrun -
Reimar Döffinger