Re: [FFmpeg-devel] [DECISION] Revoke the decision of dropping ffserver
On 28.11.2016 19:53, Lou Logan wrote: On Mon, Nov 28, 2016, at 09:15 AM, Nicolas George wrote: ffserver has users I don't know of any. Do you have an estimation of how many users there may be? How much feedback has there been from these alleged users regarding the removal plans? I don't have an estimation, but some time ago I got emails from users directly into my email regarding some issues or asking for help. It was the time when I worked on config stuff of ffserver. I am not albe to tell you if it was 2, 3 or more, but there are some users for sure. Regarding voting I will not vote, but arguing ffserver must be deleted, because it was decided basing on facts that may change soon is funny. Of course it is an important rule of the universe that in regular periods of time very determined person to delete something is spawned (or to block something), but being reasonable is also good rule of thumb :P ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] Remove the ffserver program and the ffm muxer/demuxer
On 27.10.2016 20:26, Michael Niedermayer wrote: On Thu, Oct 27, 2016 at 11:03:07AM -0700, Reynaldo H. Verdejo Pinochet wrote: I agree with moving the apps to a seperate repo hosted within the same infra and keeping ffserver. I will help with ffserver as my time & todo list permits I don't follow ffmpeg list for long time, so please forgive me if I said something already discussed, but the news said it is removed because of cleanups. That is reasonable, but in such case moving it to separate repo is nonsense. Also I'm not sure ffmenc/dec removal is good decision at the moment. I don't want to suggest there is ohter app than ffserver that use it, but with these removed you can forget ffserver will exists in other repo, right? ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 0/5] Add SDL2 support & drop SDL1 support
On 24 September 2016 at 18:17, Josh de Kock wrote: > On 22/09/2016 20:28, Josh de Kock wrote: > >> Hopefully the final iteration of this set--it drops SDL1 support >> entirely rather than adding another ffplay version and keeping the >> SDL1 output device. All of it has been reviewed before, so there >> shouldn't be too much (or anything to change). >> >> Josh de Kock (3): >> lavd: Add SDL2 output device >> MAINTAINERS: update my entries >> lavd: drop SDL1 device & drop SDL1 support in general >> >> Lukasz Marek (1): >> lavd/opengl: use SDL2 >> >> Marton Balint (1): >> ffplay: add SDL2 support >> >> Changelog| 4 + >> MAINTAINERS | 1 + >> configure| 51 ++-- >> ffplay.c | 594 -- >> - >> libavdevice/Makefile | 2 +- >> libavdevice/alldevices.c | 2 +- >> libavdevice/opengl_enc.c | 109 - >> libavdevice/sdl.c| 377 -- >> libavdevice/sdl2.c | 377 ++ >> 9 files changed, 702 insertions(+), 815 deletions(-) >> delete mode 100644 libavdevice/sdl.c >> create mode 100644 libavdevice/sdl2.c >> >> > Set applied. > You ignored my review (at least part of it) regarding sdl2 device. http://ffmpeg.org/pipermail/ffmpeg-devel/2016-September/199732.html ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] Dropping SDL1 support [VOTE]
On Sep 18, 2016 04:57, "Ronald S. Bultje" wrote: > > Hi, > > On Sat, Sep 17, 2016 at 4:26 PM, Marton Balint wrote: > > > I think we should wait until Lukasz does the port of the opengl device to > > SDL2, and do the switch after that. > > > When do we think that'll be finished? I missed this, I alredy posted a patch, it needs just one line fix so not consider it as a blocker. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/4] lavd: Add SDL2 output device
On 15.09.2016 00:27, Josh de Kock wrote: Acked-by: Michael Niedermayer Signed-off-by: Josh de Kock --- configure| 28 +++- libavdevice/Makefile | 1 + libavdevice/alldevices.c | 1 + libavdevice/sdl2.c | 377 +++ 4 files changed, 406 insertions(+), 1 deletion(-) create mode 100644 libavdevice/sdl2.c +typedef struct { +AVClass *class; +SDL_Window *window; +SDL_Renderer *renderer; +char *window_title; +int window_width, window_height; /**< size of the window */ +int window_fullscreen; +int window_borderless; + +SDL_Texture *texture; +int texture_fmt; +SDL_Rect texture_rect; + +int inited; +SDL_Thread *event_thread; +SDL_mutex *mutex; +SDL_cond *init_cond; these 3 are unused +int quit; this also can be removed +} SDLContext; + + +static int sdl2_write_trailer(AVFormatContext *s) +{ +SDLContext *sdl = s->priv_data; + +sdl->quit = 1; as there is no other thread then it is redundant +if (sdl->texture) +SDL_DestroyTexture(sdl->texture); +sdl->texture = NULL; +if (sdl->event_thread) +SDL_WaitThread(sdl->event_thread, NULL); +sdl->event_thread = NULL; +if (sdl->mutex) +SDL_DestroyMutex(sdl->mutex); +sdl->mutex = NULL; +if (sdl->init_cond) +SDL_DestroyCond(sdl->init_cond); +sdl->init_cond = NULL; + +if (sdl->renderer) +SDL_DestroyRenderer(sdl->renderer); +sdl->renderer = NULL; + +if (sdl->window) +SDL_DestroyWindow(sdl->window); +sdl->window = NULL; + +if (!sdl->inited) +SDL_Quit(); are you sure this is OK? + +return 0; +} + +#define SDL_BASE_FLAGS (SDL_SWSURFACE|SDL_WINDOW_RESIZABLE) + +static int sdl2_write_header(AVFormatContext *s) +{ +SDLContext *sdl = s->priv_data; +AVStream *st = s->streams[0]; +AVCodecParameters *codecpar = st->codecpar; +int i, ret = 0; + +if (!sdl->window_title) +sdl->window_title = av_strdup(s->filename); + +if (SDL_WasInit(SDL_INIT_VIDEO)) { +av_log(s, AV_LOG_WARNING, + "SDL video subsystem was already inited, you could have multiple SDL outputs. This may cause unknown behaviour.\n"); +sdl->inited = 1; +} + +if ( s->nb_streams > 1 +|| codecpar->codec_type != AVMEDIA_TYPE_VIDEO +|| codecpar->codec_id != AV_CODEC_ID_RAWVIDEO) { +av_log(s, AV_LOG_ERROR, "Only supports one rawvideo stream\n"); +goto fail; +} + +for (i = 0; sdl_texture_format_map[i].format != AV_PIX_FMT_NONE; i++) { +if (sdl_texture_format_map[i].format == codecpar->format) { +sdl->texture_fmt = sdl_texture_format_map[i].texture_fmt; +break; +} +} + +if (!sdl->texture_fmt) { +av_log(s, AV_LOG_ERROR, + "Unsupported pixel format '%s', choose one of yuv420p, yuyv422, uyvy422, BGRA\n", This log message is not clear. It made sense where there was only 3 planar format. I would remove second part + av_get_pix_fmt_name(codecpar->format)); +goto fail; +} + +/* resize texture to width and height from the codec context information */ +int flags; +flags = SDL_BASE_FLAGS | (sdl->window_fullscreen ? SDL_WINDOW_FULLSCREEN : 0) | + (sdl->window_borderless ? SDL_WINDOW_BORDERLESS : 0); + +/* initialization */ +if (!sdl->inited){ +if (SDL_Init(SDL_INIT_VIDEO) != 0) { +av_log(s, AV_LOG_ERROR, "Unable to initialize SDL: %s\n", SDL_GetError()); +goto fail; +} +} + +sdl->window_width = sdl->texture_rect.w = codecpar->width; +sdl->window_height = sdl->texture_rect.h = codecpar->height; +sdl->texture_rect.x = sdl->texture_rect.y = 0; + +if (SDL_CreateWindowAndRenderer(sdl->window_width, sdl->window_height, +flags, &sdl->window, &sdl->renderer) != 0){ +av_log(sdl, AV_LOG_ERROR, "Couldn't create window and renderer: %s\n", SDL_GetError()); +goto fail; +} + +SDL_SetWindowTitle(sdl->window, sdl->window_title); + +sdl->texture = SDL_CreateTexture(sdl->renderer, sdl->texture_fmt, SDL_TEXTUREACCESS_STREAMING, + sdl->window_width, sdl->window_height); + +if (!sdl->texture) { +av_log(sdl, AV_LOG_ERROR, "Unable to set create mode: %s\n", SDL_GetError()); +goto fail; +} + +av_log(s, AV_LOG_VERBOSE, "w:%d h:%d fmt:%s -> w:%d h:%d\n", + codecpar->width, codecpar->height, av_get_pix_fmt_name(codecpar->format), + sdl->window_width, sdl->window_height); + +sdl->inited = 1; + +return 0; +fail: +sdl2_write_trailer(s); +return ret; +} + +static int sdl2_write_packet(AVFormatContext *s, AVPacket *pkt) +{ +int ret = 0; +SDLContext *sdl = s->priv_data; +AVCodecParameters *codecpar = s->streams[0]-
Re: [FFmpeg-devel] [PATCH] lavd/opengl: use SDL2
On 18.09.2016 10:04, Josh de Kock wrote: On 18/09/2016 05:51, Lukasz Marek wrote: W dniu niedziela, 18 września 2016 Lou Logan > napisał(a): On Sat, Sep 17, 2016, at 05:21 PM, Lukasz Marek wrote: On 18 September 2016 at 00:30, Josh de Kock wrote: [...] Would like some feedback where it doesnt work etc [...] I said I will patch opengl, your implemantation donsn't look good at this point, declined, It would be helpful to provide some details describing what exactly does not look good. ___ opengl device depends on opengl context. this patch works on sdl renderer which works on X. in many cases X is opengł so patch is working for many cases, but is shity in general. Sorry, I don't understand. Could you rephrase this? I updated my version. I wanted to submit after vote is finished, but to save our time you can review it and include in your sdl patchset. >From c8988099c8535c77382b6f05d23326a0270bb2f4 Mon Sep 17 00:00:00 2001 From: Lukasz Marek Date: Sun, 18 Sep 2016 19:13:12 +0200 Subject: [PATCH] lavd/opengl: use SDL2 Signed-off-by: Lukasz Marek --- libavdevice/opengl_enc.c | 109 +-- 1 file changed, 49 insertions(+), 60 deletions(-) diff --git a/libavdevice/opengl_enc.c b/libavdevice/opengl_enc.c index 1dbbb80..94259a2 100644 --- a/libavdevice/opengl_enc.c +++ b/libavdevice/opengl_enc.c @@ -46,7 +46,7 @@ #include #endif -#if HAVE_SDL +#if HAVE_SDL2 #include #endif @@ -174,8 +174,9 @@ static const GLushort g_index[6] = typedef struct OpenGLContext { AVClass *class;///< class for private options -#if HAVE_SDL -SDL_Surface *surface; +#if HAVE_SDL2 +SDL_Window *window; +SDL_GLContext glcontext; #endif FFOpenGLFunctions glprocs; @@ -341,30 +342,14 @@ static int opengl_control_message(AVFormatContext *h, int type, void *data, size return AVERROR(ENOSYS); } -#if HAVE_SDL -static int opengl_sdl_recreate_window(OpenGLContext *opengl, int width, int height) -{ -opengl->surface = SDL_SetVideoMode(width, height, - 32, SDL_OPENGL | SDL_RESIZABLE); -if (!opengl->surface) { -av_log(opengl, AV_LOG_ERROR, "Unable to set video mode: %s\n", SDL_GetError()); -return AVERROR_EXTERNAL; -} -SDL_GL_SetAttribute(SDL_GL_RED_SIZE, 8); -SDL_GL_SetAttribute(SDL_GL_GREEN_SIZE, 8); -SDL_GL_SetAttribute(SDL_GL_BLUE_SIZE, 8); -SDL_GL_SetAttribute(SDL_GL_ALPHA_SIZE, 8); -SDL_GL_SetAttribute(SDL_GL_DOUBLEBUFFER, 1); -return 0; -} - +#if HAVE_SDL2 static int opengl_sdl_process_events(AVFormatContext *h) { -int ret; OpenGLContext *opengl = h->priv_data; +AVDeviceRect message; SDL_Event event; SDL_PumpEvents(); -while (SDL_PeepEvents(&event, 1, SDL_GETEVENT, SDL_ALLEVENTS) > 0) { +while (SDL_PeepEvents(&event, 1, SDL_GETEVENT, SDL_FIRSTEVENT, SDL_LASTEVENT) > 0) { switch (event.type) { case SDL_QUIT: return AVERROR(EIO); @@ -375,23 +360,14 @@ static int opengl_sdl_process_events(AVFormatContext *h) return AVERROR(EIO); } return 0; -case SDL_VIDEORESIZE: { -char buffer[100]; -int reinit; -AVDeviceRect message; -/* clean up old context because SDL_SetVideoMode may lose its state. */ -SDL_VideoDriverName(buffer, sizeof(buffer)); -reinit = !av_strncasecmp(buffer, "quartz", sizeof(buffer)); -if (reinit) { -opengl_deinit_context(opengl); -} -if ((ret = opengl_sdl_recreate_window(opengl, event.resize.w, event.resize.h)) < 0) -return ret; -if (reinit && (ret = opengl_init_context(opengl)) < 0) -return ret; -message.width = opengl->surface->w; -message.height = opengl->surface->h; -return opengl_control_message(h, AV_APP_TO_DEV_WINDOW_SIZE, &message, sizeof(AVDeviceRect)); +case SDL_WINDOWEVENT: +switch(event.window.event) { +case SDL_WINDOWEVENT_RESIZED: +case SDL_WINDOWEVENT_SIZE_CHANGED: +SDL_GL_GetDrawableSize(opengl->window, &message.width, &message.height); +return opengl_control_message(h, AV_APP_TO_DEV_WINDOW_SIZE, &message, sizeof(AVDeviceRect)); +default: +break; } } } @@ -400,23 +376,34 @@ static int opengl_sdl_process_events(AVFormatContext *h) static int av_cold opengl_sdl_create_window(AVFormatContext *h) { -int ret; -char buffer[100]; OpenGLContext *opengl = h->priv_data; AVDeviceRect message; if (SDL_Init(SDL_INIT_VIDEO)) { av_log(opengl, AV_LOG_ERROR, "Unable to initialize SDL:
Re: [FFmpeg-devel] [PATCH] lavd/opengl: use SDL2
On 18.09.2016 10:04, Josh de Kock wrote: On 18/09/2016 05:51, Lukasz Marek wrote: W dniu niedziela, 18 września 2016 Lou Logan > napisał(a): On Sat, Sep 17, 2016, at 05:21 PM, Lukasz Marek wrote: On 18 September 2016 at 00:30, Josh de Kock wrote: [...] Would like some feedback where it doesnt work etc [...] I said I will patch opengl, your implemantation donsn't look good at this point, declined, It would be helpful to provide some details describing what exactly does not look good. ___ opengl device depends on opengl context. this patch works on sdl renderer which works on X. in many cases X is opengł so patch is working for many cases, but is shity in general. Sorry, I don't understand. Could you rephrase this? To make opengl work it needs a context. Could you point where you create it? ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] lavd/opengl: use SDL2
W dniu niedziela, 18 września 2016 Lou Logan > napisał(a): > On Sat, Sep 17, 2016, at 05:21 PM, Lukasz Marek wrote: > > On 18 September 2016 at 00:30, Josh de Kock wrote: > [...] > >> Would like some feedback where it doesnt work etc > [...] > > I said I will patch opengl, your implemantation donsn't look good at this > > point, declined, > > It would be helpful to provide some details describing what exactly does > not look good. > ___ opengl device depends on opengl context. this patch works on sdl renderer which works on X. in many cases X is opengł so patch is working for many cases, but is shity in general. > > ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] lavd/opengl: use SDL2
On 18 September 2016 at 00:30, Josh de Kock wrote: > I did this in about 5 minutes, and only tested it with one sample (on > OSX using the cocoa opengl renderer). Seems to work, but probably > won't for all cases. Would like some feedback where it doesnt work etc > > Signed-off-by: Josh de Kock > --- > libavdevice/opengl_enc.c | 87 +- > -- > 1 file changed, 46 insertions(+), 41 deletions(-) > > diff --git a/libavdevice/opengl_enc.c b/libavdevice/opengl_enc.c > index 1dbbb80..1917459 100644 > --- a/libavdevice/opengl_enc.c > +++ b/libavdevice/opengl_enc.c > @@ -46,7 +46,7 @@ > #include > #endif > > -#if HAVE_SDL > +#if HAVE_SDL2 > #include > #endif > > @@ -174,8 +174,10 @@ static const GLushort g_index[6] = > typedef struct OpenGLContext { > AVClass *class;///< class for private options > > -#if HAVE_SDL > -SDL_Surface *surface; > +#if HAVE_SDL2 > +SDL_Texture *texture; > +SDL_Window *window; > +SDL_Renderer *renderer; > I said I will patch opengl, your implemantation donsn't look good at this point, declined, ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] Resurrection of ffserver
On 17.09.2016 23:10, Rostislav Pehlivanov wrote: On 17 September 2016 at 22:04, Lukasz Marek wrote: Yeah, no. You can hardly say ffserver is a perfect program right now, so really moving it to another repo is hardly much of a change in terms of functionality. Bisection won't matter if half the program needs to be rewritten to make it good. And the future of ffm mux/demux is the same as the future of ffserver. And besides none of this even matters because ffserver's getting deleted. You can't stop it. This was decided long ago. It's in the news. It won't get better overnight. This email thread is now about whether someone wants to maintain it somewhere else, which doesn't concern the project other than having people who are interested in contributing to something like that. I didn't know it is decided to delete it. In such case I have no objections. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] Resurrection of ffserver
On 17.09.2016 22:27, Rostislav Pehlivanov wrote: On 17 September 2016 at 20:58, Michael Fritscher < michael.fritsc...@telematik-zentrum.de> wrote: Good day, I read the sad news about ffservers a few days ago and have already written some mails to ffmpeg-user. I would like to step in to make ffserver maintainable and distributable again, especially because of the many users depending on ffserver - including myself. I'm capable of C and had made some tiny changes to ffmpeg already, but I'm a beginner regarding the architecture and "unwritten rules" of ffmpeg development. Additionally, I would like to discuss strategic decisions with the main developers to avoid useless work. I really think ffserver is better off being in a separate repository. That way there'd be some finer attention brought to keeping the API usable by external users too. Same with the rest of the programs. So yeah, if you want, just make a separate repository somewhere with only ffserver.c and a configure + makefile and try to make something better and usable. I don't want to declare my statement here what should be done, but there is one more thing you didn't mention. ffserver depends on some specific components located inside ffmpeg libraries. At least on ffm mux/demux. They cannot be moved. The problem is, people sometimes opens ticket with error report like that: "I used version X, released about year ago and it worked fine. Now I upgraded and it doesn't work" Keeping it everything in one repo allows you to use bisection to find where the problem were introduced. Where you keep it in separate repositories it become much more complex. Now when you move back in repo history, you know all tools will compile (unless some commits are pushed in wrong order), when you have more repositories then you have to correlate it. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] Dropping SDL1 support [VOTE]
On 15.09.2016 23:36, Josh de Kock wrote: On 15/09/2016 22:28, Lukasz Marek wrote: On 15.09.2016 23:19, Moritz Barsnick wrote: On Thu, Sep 15, 2016 at 14:36:32 -0300, James Almer wrote: * SDL1 is old and effectively unmaintained. I understand this verbatim, but what is it supposed to mean? Has SDL1 gotten rotten? Or more precisely: - Has technology evolved, and the library not kept up? - Is a long list of bugs (somehow effecting ffmpeg) piling up and never being fixed? - Something else? It doesn't compile on Mac OS I've been testing it just fine on Mac OS. I don't remember now everything, I worked with it like 2 years ago, but I got the same issue as here https://forums.libsdl.org/viewtopic.php?p=45264&sid=ae0efc2e82d373067c5112bfba70cd11 Other example is that SDL is trying to use X11 on Mac which is dropped (since 10.8?). You can ignore my answer as it doesn't stand for anything. It won't stop introducing SDL 2.0 nor force 1.2 drop, but asked question if SDL 1.2 is getting rotten then asnwer is yes IMHO. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] Dropping SDL1 support [VOTE]
On 15.09.2016 23:19, Moritz Barsnick wrote: On Thu, Sep 15, 2016 at 14:36:32 -0300, James Almer wrote: * SDL1 is old and effectively unmaintained. I understand this verbatim, but what is it supposed to mean? Has SDL1 gotten rotten? Or more precisely: - Has technology evolved, and the library not kept up? - Is a long list of bugs (somehow effecting ffmpeg) piling up and never being fixed? - Something else? It doesn't compile on Mac OS ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH v3 3/3] lavd: deprecate opengl outdev
On 14.09.2016 19:53, Nicolas George wrote: Le sextidi 26 fructidor, an CCXXIV, Josh de Kock a écrit : This device depends on SDL which is deprecated. Signed-off-by: Josh de Kock If I understand correctly, SDL is only used in this device to set up a window and a context in a cross-platform way. I even notice that the code can be used without SDL altogether if the applications handles the context and window itself. You are correct. SDL is used just to created default window with opengl context. Probably the only use case of this is to test this device with following command: ffmpeg -i video -f opengl aaa. The relevant implementation is depending on opengl API only and has nothing to do with SDL. Removing it just because of switching to SDL 2.0 is misunderstanding. I can port SDL part to use 2.0 API, just figure out how this will be handled. I mean if you want to support both or just 2.0. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] FTP graceful close data connection to avoid server abort
On 02.06.2016 14:29, Camille Gonnet wrote: When writing files to FTP, if the data connection is closed before the control connection, the server may handle it as an aborted file transfer and create and leave the file empty. --- libavformat/ftp.c | 14 ++ 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/libavformat/ftp.c b/libavformat/ftp.c index 0663b47..00747bb 100644 --- a/libavformat/ftp.c +++ b/libavformat/ftp.c @@ -220,15 +220,21 @@ static int ftp_send_command(FTPContext *s, const char *command, static void ftp_close_data_connection(FTPContext *s) { -ffurl_closep(&s->conn_data); +static const int close_codes[] = {225, 226, 0}; + +if (s->conn_data) { +ffurl_closep(&s->conn_data); +// Need to wait for status, or file transfer might be aborted on server side +ftp_status(s, NULL, close_codes); +} s->position = 0; s->state = DISCONNECTED; } This is not working. ./ffplay ftp://user:pass@localhost/1.avi hangs at startup. It was working for you because (probably) your writing operation didn't perform seeking (did you enabled ftp-write-seekable?). During seek operation, when seek is really done, ftp_abort is called and there is ftp_close_data_connection called with status check followed. So status is checked twice while sent by server just once. I can work it out, but just tell what server has this issue. Perfectly setup some account for me. You can of course work it out yourself, but please test also other scenarios and other servers. First thing to check is to revert patch completely and replace ftp_close_both_connections by ftp_abort inside ftp_close function. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] FTP graceful close data connection to avoid server abort
On 09.06.2016 00:25, Michael Niedermayer wrote: > On Thu, Jun 02, 2016 at 02:29:47PM +0200, Camille Gonnet wrote: >> When writing files to FTP, if the data connection is closed before the >> control connection, the server may handle it as an aborted file transfer >> and create and leave the file empty. > > which ftp server, or is that in the RFC, if so please refer to it RFC is good for reference, unfortunately even popular linux implementations don't follow it strictly (sometimes it is hard to say they follow at all some aspects). Regarding aborting and stuff it is really implementation dependent. I can't remember now which implementation and what version, but one was totally unresponsive on control connection while active transfer on data connection, so the only way to abort it was to close data connection. Reverting order of closing should be OK, but I'm not really sure expecting all implementation to send 225/226 code is correct. I would suggest to at least check state if it is UPLOADING and apply that change only for this case. Perfectly this behaviour could be enabled by an option and autodetected in ftp_connect_control_connection when server send its name (note 220 code's message may be overwritten so an option to enforce is needed) ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] Remove Derek Buitenhuis from MAINTAINERS
On 19 May 2016 at 15:18, Derek Buitenhuis wrote: > On 5/19/2016 2:12 PM, Michael Niedermayer wrote: > > if derek still wants to leave in 2 weeks then so be it, its his choice > > but i really hope things can be resolved in a way that everyone > > stays and works together and is happy > > I will wait 2 weeks. > Is Derek revoked to commit or what? Couldn't he just commit this patch and leave? :P I was a problem for some people, but I see they still have problems. Let people with problems go away with they problems. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] ffplay: fix silence insertion on error or pause
On 03.04.2016 21:37, Marton Balint wrote: > Insertion of silence was a bit broken since > df34b700981de606ca4847e1ed0bfdf9ac3e9104 because the info whether or not the > source buffer supposed to be silence must be kept between callbacks. Failing > to > do so causes rogue samples from the last buffer to be presented, I guess even > a > crash can occur under some circumstances. > > This patch uses a NULL audio_buf to keep the silence state across audio > callbacks. > > Signed-off-by: Marton Balint > --- > ffplay.c | 8 > 1 file changed, 4 insertions(+), 4 deletions(-) mea culpa LGFM ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] GSoC again
On 23.03.2016 17:09, Gerion Entrup wrote: > 3. Implement adaptive switching. > > A note to the 3rd point. I'm absolutely not sure, what amount of work this is > (maybe you could comment it) and what the best place would be to implement > this (including whether this whole feature is meaningful). I would say, this > could > be either in the demuxer itself or in ffplay (can decide a muxer such a thing, > like bandwidth?). I think it is not possible to be done inside demuxer. Different qualities may have different metadata such as width, height, maybe different codec. HLS is exposing all qualities as separate streams and player may choose the one it needs. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] ffplay: remove redundant silence buffer
On 19.03.2016 19:16, Marton Balint wrote: > > On Thu, 17 Mar 2016, Lukasz Marek wrote: > >> Signed-off-by: Lukasz Marek >> --- >> ffplay.c | 13 ++--- >> 1 file changed, 6 insertions(+), 7 deletions(-) >> > LGTM, thanks. Ok, pushed, thx ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] ffplay: remove redundant silence buffer
Signed-off-by: Lukasz Marek --- ffplay.c | 13 ++--- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/ffplay.c b/ffplay.c index 2cfdf26..731ae25 100644 --- a/ffplay.c +++ b/ffplay.c @@ -243,7 +243,6 @@ typedef struct VideoState { AVStream *audio_st; PacketQueue audioq; int audio_hw_buf_size; -uint8_t silence_buf[SDL_AUDIO_MIN_BUFFER_SIZE]; uint8_t *audio_buf; uint8_t *audio_buf1; unsigned int audio_buf_size; /* in bytes */ @@ -2532,7 +2531,7 @@ static int audio_decode_frame(VideoState *is) static void sdl_audio_callback(void *opaque, Uint8 *stream, int len) { VideoState *is = opaque; -int audio_size, len1; +int audio_size, len1, silence = 0; audio_callback_time = av_gettime_relative(); @@ -2541,8 +2540,8 @@ static void sdl_audio_callback(void *opaque, Uint8 *stream, int len) audio_size = audio_decode_frame(is); if (audio_size < 0) { /* if error, just output silence */ - is->audio_buf = is->silence_buf; - is->audio_buf_size = sizeof(is->silence_buf) / is->audio_tgt.frame_size * is->audio_tgt.frame_size; + silence = 1; + is->audio_buf_size = SDL_AUDIO_MIN_BUFFER_SIZE / is->audio_tgt.frame_size * is->audio_tgt.frame_size; } else { if (is->show_mode != SHOW_MODE_VIDEO) update_sample_display(is, (int16_t *)is->audio_buf, audio_size); @@ -2553,11 +2552,11 @@ static void sdl_audio_callback(void *opaque, Uint8 *stream, int len) len1 = is->audio_buf_size - is->audio_buf_index; if (len1 > len) len1 = len; -if (!is->muted && is->audio_volume == SDL_MIX_MAXVOLUME) +if (!is->muted && !silence && is->audio_volume == SDL_MIX_MAXVOLUME) memcpy(stream, (uint8_t *)is->audio_buf + is->audio_buf_index, len1); else { -memset(stream, is->silence_buf[0], len1); -if (!is->muted) +memset(stream, 0, len1); +if (!is->muted && !silence) SDL_MixAudio(stream, (uint8_t *)is->audio_buf + is->audio_buf_index, len1, is->audio_volume); } len -= len1; -- 1.9.1 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 3/3] avformat: add youtube-dl based demuxer
On 09.04.2015 13:50, Kieran Kunhya wrote: On 9 April 2015 at 12:17, Rodger Combs wrote: Agreed, this belongs in a higher layer. I think it'd be reasonable for FFmpeg to have a higher-layer library handling things like playlists and this, but that's another conversation. +1 So instead of complaining and spamming with "+1", prepare a patch and submit it? Community will discuss it and do what's the best for the project (and yes, project is not single person who has their wishes). Personally I woulnt complain about that separation if done properly, but just don't see a reason for it. Most of the stuff you don't need/like you can disable at compilation level. We have separation libavformat/libavdevice and most of patches I submitted regarding libavdevice were complained by one person. Sometimes I have a feeling that ffmpeg is an open source project to feed wm4's project. Everything he doesn't need is "retarded", "rotten", "dumb", etc, etc. I don't want to be drama queen here, but the way author of this patch was treated is bellow any level of dignity. FFmpeg has more to offer that show how many douches is subscribed to the mailing list. Unless these things are handled don't treat me as member of this "community" anymore. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 3/3] avformat: add youtube-dl based demuxer
> Easy. Just merge webkit into ffmpeg. It's for the greater good! Any specific reason for webkit in ffmpeg? You said something about not being trolling. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 3/3] avformat: add youtube-dl based demuxer
W dniu środa, 8 kwietnia 2015 Gilles Chanteperdrix < gilles.chanteperd...@xenomai.org> And forgot to add my apologies for these bad patches and the noise > that followed. > No need for that. Your demuxer seems to use public API only so you can try to move this implementation to your project and register this demuxer from your application. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] Segmentation fault in libquvi.c
On 8 April 2015 at 18:07, Michael Niedermayer wrote: > On Wed, Apr 08, 2015 at 06:02:16PM +0200, wm4 wrote: > > On Wed, 8 Apr 2015 17:52:18 +0200 > > Michael Niedermayer wrote: > > > > > On Wed, Apr 08, 2015 at 05:40:17PM +0200, Gilles Chanteperdrix wrote: > > > > On Wed, Apr 08, 2015 at 05:29:13PM +0200, wm4 wrote: > > > > > On Wed, 8 Apr 2015 17:22:58 +0200 > > > > > Gilles Chanteperdrix wrote: > > > > > > > > > > > On Wed, Apr 08, 2015 at 04:13:58PM +0200, wm4 wrote: > > > > > > > On Wed, 8 Apr 2015 15:37:03 +0200 > > > > > > > Gilles Chanteperdrix wrote: > > > > > > > > > > > > > > > On Wed, Apr 08, 2015 at 02:40:52PM +0200, Michael > Niedermayer wrote: > > > > > > > > > On Wed, Apr 08, 2015 at 10:55:45AM +0200, Gilles > Chanteperdrix wrote: > > > > > > > > > > Hi, > > > > > > > > > > > > > > > > > > > > I just triend libquvi, and get a segmentation fault in > the > > > > > > > > > > libquvi_read_header function, because ff_copy_whitelists > is called > > > > > > > > > > before qc->fmtctx is allocated by avformat_open_input. I > added a > > > > > > > > > > call to avformat_alloc_context() before > ff_copy_whitelists and the > > > > > > > > > > libquvi demuxer works. > > > > > > > > > > > > > > > > > > > > However, I wonder how to fix this properly: the error > handling > > > > > > > > > > labels look backward, so that I am not sure where to > free the > > > > > > > > > > allocated context in case of error. > > > > > > > > > > > > > > > > > > applied this, yes its correct > > > > > > > > > > > > > > > > Ok, there are other details missing, the stream does not get > a > > > > > > > > duration, start_time and bitrate. This can easily be fixed, > but as > > > > > > > > wm4 said libquvi seems an abandoned project. > > > > > > > > > > > > > > > > Would there be any interest in a solution based on > youtube-dl? It > > > > > > > > seems to be the standard, these days. I just ran a few tests: > > > > > > > > > > > > > > > > youtube -qs 'url' returns 0 or 1 depending on whether the > url can be > > > > > > > > parsed by the tool > > > > > > > > > > > > > > > > youtube -e 'url' prints the stream title > > > > > > > > > > > > > > > > and youtube -f 'url' prints the video url. > > > > > > > > > > > > > > > > To use this portably, we can use system() and redirect the > > > > > > > > output to a temporary file, and read the title or URL from > the file. > > > > > > > > Or is popen available on all platforms where ffmpeg runs? > > > > > > > > > > > > > > > > Is it a clean enough solution? If yes, I can submit the > patch adding > > > > > > > > this solution. From what I could see, all solutions to parse > youtube > > > > > > > > (including quvi) are based on scripts anyway. > > > > > > > > > > > > > > > > > > > > > > Calling external processes from within libavformat doesn't > sound like > > > > > > > the right thing to do. You can do it perfectly fine in a > higher level, > > > > > > > of course (I'm doing this too). > > > > > > > > > > > > The point is: the youtube-dl developers are doing a terrific job > > > > > > maintaining their software and supporting a lot of sites: > > > > > > run youtube-dl --list-extractors to be convinced. So, I do not > think > > > > > > it makes sense to duplicate this effort "at a higher level", > this is > > > > > > a waste of ressources. Maybe ask them kindly to provide us with > some > > > > > > command line arguments that would arrange us would make sense, > > > > > > though. > > > > > > > > > > > > > > > > I'm not talking about duplicating this. I'm saying that you should > put > > > > > calling youtube-dl not into libavformat, but into the application > which > > > > > uses libavformat. > > > > > > > > So, everyone should duplicate the job of using youtube-dl instead of > > > > having it done once and for all in a common library ? > > > > > > no, some solution should be found so that all users of the ffmpeg libs > > > have this support and do not need to duplicate code. > > > > > > The solution should be choosen so as to make people be happy about it > > > and not didlike the design. > > > > > > a libyoutube-dl might be an option, i dont know > > > > > > [...] > > > > What the heck would this lib consist of? > > > > 3 lines of code that call popen()? > > if thats what makes everyone happy then that would be possible. > of course it would be better if it interfaced to youtube-dl in a > nicer way > maybe https://docs.python.org/2/extending/embedding.html ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] Segmentation fault in libquvi.c
On 8 April 2015 at 15:37, Gilles Chanteperdrix < gilles.chanteperd...@xenomai.org> wrote: > On Wed, Apr 08, 2015 at 02:40:52PM +0200, Michael Niedermayer wrote: > > On Wed, Apr 08, 2015 at 10:55:45AM +0200, Gilles Chanteperdrix wrote: > > > Hi, > > > > > > I just triend libquvi, and get a segmentation fault in the > > > libquvi_read_header function, because ff_copy_whitelists is called > > > before qc->fmtctx is allocated by avformat_open_input. I added a > > > call to avformat_alloc_context() before ff_copy_whitelists and the > > > libquvi demuxer works. > > > > > > However, I wonder how to fix this properly: the error handling > > > labels look backward, so that I am not sure where to free the > > > allocated context in case of error. > > > > applied this, yes its correct > > Ok, there are other details missing, the stream does not get a > duration, start_time and bitrate. This can easily be fixed, but as > wm4 said libquvi seems an abandoned project. > > Would there be any interest in a solution based on youtube-dl? It > seems to be the standard, these days. I just ran a few tests: > > youtube -qs 'url' returns 0 or 1 depending on whether the url can be > parsed by the tool > > youtube -e 'url' prints the stream title > > and youtube -f 'url' prints the video url. > > To use this portably, we can use system() and redirect the > output to a temporary file, and read the title or URL from the file. > Or is popen available on all platforms where ffmpeg runs? > > Is it a clean enough solution? If yes, I can submit the patch adding > this solution. From what I could see, all solutions to parse youtube > (including quvi) are based on scripts anyway. > I submitted a patch more that year ago to support youtube natively, as I remember this was based on the script you mention, but it was rejected because we have libquvi. [sic!] Project not actively developed is not the same as project not working. In corner case it is not developed because everything is working perfectly. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 2/3] ffplay: add basic credentials handler
Signed-off-by: Lukasz Marek --- ffplay.c | 63 ++- 1 file changed, 62 insertions(+), 1 deletion(-) diff --git a/ffplay.c b/ffplay.c index 8fa8566..692773e 100644 --- a/ffplay.c +++ b/ffplay.c @@ -29,6 +29,9 @@ #include #include #include +#if HAVE_TERMIOS_H +#include +#endif #include "libavutil/avstring.h" #include "libavutil/colorspace.h" @@ -1690,7 +1693,7 @@ display: } } is->force_refresh = 0; -if (show_status) { +if (show_status && is->ic) { static int64_t last_time; int64_t cur_time; int aqsize, vqsize, sqsize; @@ -3781,6 +3784,62 @@ static int lockmgr(void **mtx, enum AVLockOp op) return 1; } +static void console_change_echo(int echo) +{ +#if HAVE_TERMIOS_H +struct termios t; +tcgetattr(0, &t); +if (echo) +t.c_lflag |= ECHO; +else +t.c_lflag &= ~ECHO; +tcsetattr(0, TCSANOW, &t); +#endif +} + +static int credetials_callback_get_val(const char *query, const char *default_val, char **value) +{ +char val[1024]; +size_t len; + +if (default_val) +printf("Enter %s [%s]: ", query, default_val); +else +printf("Enter %s: ", query); +fflush(stdout); +fgets(val, sizeof(val), stdin); +len = strlen(val); +if (len > 1) { +av_freep(value); +val[len - 1] = '\0'; //remove trailing \n +*value = av_strdup(val); +if (!*value) +return AVERROR(ENOMEM); +} +return 0; +} + +static int credetials_callback_func(AVIOCredentials *credentials, void *opaque) +{ +int ret = 0; + +printf("Provide credentials for URL: %s\n", credentials->url); +if (credentials->username) +ret = credetials_callback_get_val("username", credentials->username, &credentials->username); +if (credentials->password && ret >= 0) { +console_change_echo(0); +ret = credetials_callback_get_val("password", NULL, &credentials->password); +console_change_echo(1); +} +printf("\n"); +return ret; +} + +static const AVIOCredentialsCB credetials_callback = { +.callback = credetials_callback_func, +.opaque = NULL, +}; + /* Called from the main */ int main(int argc, char **argv) { @@ -3801,6 +3860,8 @@ int main(int argc, char **argv) av_register_all(); avformat_network_init(); +avio_register_default_credentials_callback(&credetials_callback); + init_opts(); signal(SIGINT , sigterm_handler); /* Interrupt (ANSI).*/ -- 1.9.1 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 1/3] [RFC]lavf/avio: add custom authentication API
Signed-off-by: Lukasz Marek --- libavformat/avio.c | 46 ++ libavformat/avio.h | 28 libavformat/url.h | 12 3 files changed, 86 insertions(+) diff --git a/libavformat/avio.c b/libavformat/avio.c index a990ea2..9006cac 100644 --- a/libavformat/avio.c +++ b/libavformat/avio.c @@ -30,9 +30,12 @@ #include "network.h" #endif #include "url.h" +#include "internal.h" static URLProtocol *first_protocol = NULL; +static AVIOCredentialsCB g_credentials_callback; + URLProtocol *ffurl_protocol_next(const URLProtocol *prev) { return prev ? prev->next : first_protocol; @@ -543,3 +546,46 @@ int ff_check_interrupt(AVIOInterruptCB *cb) return ret; return 0; } + +void avio_register_default_credentials_callback(const AVIOCredentialsCB *callback) +{ +if (callback) +g_credentials_callback = *callback; +} + +int ff_url_get_credentials(const URLContext *h, AVIOCredentials *credentials) +{ +char url[MAX_URL_SIZE], proto[MAX_URL_SIZE], host[MAX_URL_SIZE]; +int port, ret = 0; + +if (!credentials) { +//probe +if (g_credentials_callback.callback || (h && h->credentials_callback.callback)) +return 0; +return AVERROR(ENOSYS); +} + +av_assert0(h); +av_url_split(proto, sizeof(proto), NULL, 0, host, sizeof(host), &port, NULL, 0, h->filename); +ff_url_join(url, sizeof(url), proto, NULL, host, port, NULL); + +av_free(credentials->url); +credentials->url = av_strdup(url); +if (!credentials->url) { +ret = AVERROR(ENOMEM); +goto fail; +} +if (h->credentials_callback.callback) +ret = h->credentials_callback.callback(credentials, h->credentials_callback.opaque); +else if (g_credentials_callback.callback) +ret = g_credentials_callback.callback(credentials, g_credentials_callback.opaque); +else +ret = AVERROR(ENOSYS); + fail: +av_freep(&credentials->url); +if (ret < 0) { +av_freep(&credentials->username); +av_freep(&credentials->password); +} +return ret; +} diff --git a/libavformat/avio.h b/libavformat/avio.h index 51913e3..60381c4 100644 --- a/libavformat/avio.h +++ b/libavformat/avio.h @@ -53,6 +53,27 @@ typedef struct AVIOInterruptCB { } AVIOInterruptCB; /** + * User provided URL credentials. Used by AVIOCredentialsCB. + * Protocol may ask for all fields or only for some of the (e.g. for password only). + * Protocol allocates required fields with default values (empty string for no default). + * Non-allocated fields must be skipped by the user. + * Allocated fields may be reallocated with their new values provided by the user. + */ +typedef struct AVIOCredentials { +char *url;/**< Read only, never include any credentials */ +char *username; /**< User's name */ +char *password; /**< User's password */ +} AVIOCredentials; + +/** + * Callback for asking user to enter URL credentials. + */ +typedef struct AVIOCredentialsCB { +int (*callback)(AVIOCredentials *credentials, void*); +void *opaque; +} AVIOCredentialsCB; + +/** * Directory entry types. */ enum AVIODirEntryType { @@ -617,4 +638,11 @@ struct AVBPrint; */ int avio_read_to_bprint(AVIOContext *h, struct AVBPrint *pb, size_t max_size); + +/** + * Register credentials callback + * @param callback callback data + */ +void avio_register_default_credentials_callback(const AVIOCredentialsCB *callback); + #endif /* AVFORMAT_AVIO_H */ diff --git a/libavformat/url.h b/libavformat/url.h index 1a845b7..f873734 100644 --- a/libavformat/url.h +++ b/libavformat/url.h @@ -47,6 +47,7 @@ typedef struct URLContext { int is_connected; AVIOInterruptCB interrupt_callback; int64_t rw_timeout; /**< maximum time to wait for (network) read/write operation completion, in mcs */ +AVIOCredentialsCB credentials_callback; } URLContext; typedef struct URLProtocol { @@ -290,5 +291,16 @@ void ff_make_absolute_url(char *buf, int size, const char *base, */ AVIODirEntry *ff_alloc_dir_entry(void); +/** + * Get user provided credentials. + * + * May be called with NULL credentials (or even protocol context) to probe implementation. + * Must not be called with NULL context when credentials is not NULL. + * + * @param h protocol context + * @param[inout] credentials credentails data, may be NULL for implementation probing. + * @return >=0 on success, negative on error + */ +int ff_url_get_credentials(const URLContext *h, AVIOCredentials *credentials); #endif /* AVFORMAT_URL_H */ -- 1.9.1 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 3/3] lavf/ftp: support credentials callback
--- libavformat/ftp.c | 96 +++ 1 file changed, 83 insertions(+), 13 deletions(-) diff --git a/libavformat/ftp.c b/libavformat/ftp.c index 27a172e..1133e02 100644 --- a/libavformat/ftp.c +++ b/libavformat/ftp.c @@ -74,6 +74,36 @@ static const AVClass ftp_context_class = { static int ftp_close(URLContext *h); +static int ftp_get_password(URLContext *h) +{ +FTPContext *s = h->priv_data; +int ret; +AVIOCredentials cretentials = { +.username = av_strdup(av_x_if_null(s->user, "anonymous")), +.password = av_strdup(av_x_if_null(s->password, "")), +}; + +if (!cretentials.username || !cretentials.password) { +ret = AVERROR(ENOMEM); +goto fail; +} + +if ((ret = ff_url_get_credentials(h, &cretentials)) < 0) +goto fail; + +av_free(s->user); +av_free(s->password); +s->user = cretentials.username; +s->password = cretentials.password; + +return 0; + + fail: +av_free(cretentials.username); +av_free(cretentials.password); +return ret; +} + static int ftp_getc(FTPContext *s) { int len; @@ -82,7 +112,7 @@ static int ftp_getc(FTPContext *s) if (len < 0) { return len; } else if (!len) { -return -1; +return AVERROR(EPIPE); } else { s->control_buf_ptr = s->control_buffer; s->control_buf_end = s->control_buffer + len; @@ -191,7 +221,7 @@ static int ftp_send_command(FTPContext *s, const char *command, if ((err = ffurl_write(s->conn_control, command, strlen(command))) < 0) return err; if (!err) -return -1; +return AVERROR(EPIPE); /* return status */ if (response_codes) { @@ -213,12 +243,13 @@ static void ftp_close_both_connections(FTPContext *s) ftp_close_data_connection(s); } -static int ftp_auth(FTPContext *s) +static int ftp_auth(URLContext *h) { +FTPContext *s = h->priv_data; char buf[CONTROL_BUFFER_SIZE]; int err; -static const int user_codes[] = {331, 230, 0}; -static const int pass_codes[] = {230, 0}; +static const int user_codes[] = {421, 331, 230, 0}; +static const int pass_codes[] = {421, 230, 0}; snprintf(buf, sizeof(buf), "USER %s\r\n", s->user); err = ftp_send_command(s, buf, user_codes, NULL); @@ -229,6 +260,10 @@ static int ftp_auth(FTPContext *s) } else return AVERROR(EACCES); } +if (err < 0) +return err; +if (err == 421) +return AVERROR(EPIPE); if (err != 230) return AVERROR(EACCES); @@ -434,6 +469,17 @@ static int ftp_restart(FTPContext *s, int64_t pos) return 0; } +static int ftp_noop(FTPContext *s) +{ +static const char *command = "TYPE I\r\n"; +static const int noop_codes[] = {200, 0}; + +if (ftp_send_command(s, command, noop_codes, NULL) != 200) +return AVERROR(EIO); + +return 0; +} + static int ftp_features(FTPContext *s) { static const char *feat_command= "FEAT\r\n"; @@ -459,6 +505,11 @@ static int ftp_connect_control_connection(URLContext *h) FTPContext *s = h->priv_data; static const int connect_codes[] = {220, 0}; +if (s->conn_control) { +/* check if connection is alive */ +if (ftp_noop(s) < 0) +ftp_close_both_connections(s); +} if (!s->conn_control) { ff_url_join(buf, sizeof(buf), "tcp", NULL, s->hostname, s->server_control_port, NULL); @@ -484,10 +535,21 @@ static int ftp_connect_control_connection(URLContext *h) } av_free(response); -if ((err = ftp_auth(s)) < 0) { -av_log(h, AV_LOG_ERROR, "FTP authentication failed\n"); -return err; -} +do { +err = ftp_auth(h); +if (err >= 0) +break; +if (err < 0 && err != AVERROR(EACCES)) { +if (err != AVERROR(EPIPE)) +av_log(h, AV_LOG_ERROR, "FTP authentication failed\n"); +return err; +} +if ((err = ftp_get_password(h)) < 0) { +if (err == AVERROR(ENOSYS)) +err = AVERROR(EACCES); +return err; +} +} while (1); if ((err = ftp_type(s)) < 0) { av_log(h, AV_LOG_ERROR, "Set content type failed\n"); @@ -575,7 +637,7 @@ static int ftp_open(URLContext *h, const char *url, int flags) char proto[10], path[MAX_URL_SIZE], credencials[MAX_URL_SIZE], hostname[MAX_URL_SIZE]; const char *tok_user = NULL, *tok_pass = NULL; char *end = NULL; -int err; +int err, retry = 100; size_t pathlen; FTPContext *s = h->priv_data; @@ -596,7 +658,7 @@ static int ftp_open(URLContext *h, const char *url, int flags) tok_pass = av_strtok(end, ":", &end); if (!tok_user) { tok_user = "anonymous"; -tok_pass = av_
[FFmpeg-devel] [PATCH 0/3] Authentication callbacks
One of the most annoying things with ffmpeg is that password must be provided as a program param. It is visible while typing and on process list. (I'm talking about ffplay/ffmpeg/ffprobe here.) This patchset provides API that protocols may use to ask user for password interactively. I added implementation in ffplay, but I see it potentially useful for any user application. The question is: is this really required, application may just ask for password when connection fails. Assuming application knows it is matter of credentials (not wrong hostname, file permissions etc), it may be ineffective. It is almost sure connection has to be established at each try. API also allows to distinguish password for reqular user/pass authentication and password for private key (not included in this patchset, but libssh may benefit from this probably). I added callback as global variable with settter and as part of URLContext. Global variable is not nice, but it allows to avoid massive attack on avio API :) I don't see any reason it cannot be done this way. I also added it to URLContext to allow muxers, demuxers (like HLS, DASH, SmoothStreaming, image2) to add a wrapper and cache these credentials as they open many urls to store sequential data (images/chunks). Patchset available at https://github.com/lukaszmluki/ffmpeg branch: auth Lukasz Marek (3): [RFC]lavf/avio: add custom authentication API ffplay: add basic credentials handler lavf/ftp: support credentials callback ffplay.c | 63 ++- libavformat/avio.c | 46 ++ libavformat/avio.h | 28 libavformat/ftp.c | 96 ++ libavformat/url.h | 12 +++ 5 files changed, 231 insertions(+), 14 deletions(-) -- 1.9.1 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] libavformat/http.c: Make http-listen work as an input stream.
On 06.04.2015 20:29, Stephan Holljes wrote: Signed-off-by: Stephan Holljes --- doc/protocols.texi | 4 +++- libavformat/http.c | 7 +-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/doc/protocols.texi b/doc/protocols.texi index 5b7b6cf..5e9173f 100644 --- a/doc/protocols.texi +++ b/doc/protocols.texi @@ -279,7 +279,9 @@ Set initial byte offset. Try to limit the request to bytes preceding this offset. @item listen -If set to 1 enables experimental HTTP server. +If set to 1 enables experimental HTTP server. This can be used to send data when +used as an output option, or read data from a client with HTTP POST when used as +an input option. @end table You can add an example, something like this: Streaming a file through network: Receiver setup: ffplay -listen 1 -i http://localhost:8181:file.mp4 Sender setup: ffmpeg -i input.mp4 -c:v copy -c:a copy http://REMOVEIP:8181/file.mp4 Nice stuff. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] examples/avio_list_dir: init/deinit network
On 05.04.2015 15:42, Michael Niedermayer wrote: On Fri, Apr 03, 2015 at 11:56:03PM +0200, Lukasz Marek wrote: Signed-off-by: Lukasz Marek --- doc/examples/avio_list_dir.c | 9 ++--- 1 file changed, 6 insertions(+), 3 deletions(-) LGTM pushed ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] libavformat/http.c: Make http-listen work as an input stream.
On Apr 5, 2015 6:02 PM, "Stephan Holljes" wrote: > > With this patch http can be used to listen for POST data to be used as an input stream. > > Signed-off-by: Stephan Holljes > --- > libavformat/http.c | 7 +-- > 1 file changed, 5 insertions(+), 2 deletions(-) This should be documented I guess. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/3] lavf/avio: Add new types to AVIODirEntryType, bump minor version
On 03.04.2015 16:19, Michael Niedermayer wrote: On Fri, Apr 03, 2015 at 03:22:29PM +0200, Nicolas George wrote: Le quartidi 14 germinal, an CCXXIII, Mariusz Szczepańczyk a écrit : diff --git a/libavformat/avio.h b/libavformat/avio.h index f61a8f8..51913e3 100644 --- a/libavformat/avio.h +++ b/libavformat/avio.h @@ -63,7 +63,10 @@ enum AVIODirEntryType { AVIO_ENTRY_NAMED_PIPE, AVIO_ENTRY_SYMBOLIC_LINK, AVIO_ENTRY_SOCKET, -AVIO_ENTRY_FILE +AVIO_ENTRY_FILE, +AVIO_ENTRY_SERVER, +AVIO_ENTRY_SHARE, +AVIO_ENTRY_WORKGROUP, }; Sorry if I missed the previous discussions on the mailing list (and if not, maybe just 8 hours before apply was a bit short for discussion), but I had a bit of a concern with this change. Until know, if you wanted to make a recursive listing, you just had to know that you had to recurse into directories. Now... should you recurse into shares? servers? workgroups? nobody knows. There should be some way of knowing whether an entry can be opened like a plain file, entered like a directory, or if it is just one of the weird things that lay in some corners of filesystems, without requiring an update when new types are added. filemode could contain a bit for directories or a function could provide this information, but for example links can be both file and directory or a new field could be added to AVIODirEntry Fortunately this is not an issue for samba as it returns link's target type. Mariusz has sent email to their mailing list with question about this strange links treatment. But this have to be handled somehow for next protocols. It cannot be a bit in filemode, because filemode may be unknown and is set to -1 in this case. I am not convinced that all enums that can be listed recursively has to be marked explicitly. If thats just a case of symlink, then extending struct with target's type and target's path may be more useful and also solves the issue. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] examples/avio_list_dir: init/deinit network
Signed-off-by: Lukasz Marek --- doc/examples/avio_list_dir.c | 9 ++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/doc/examples/avio_list_dir.c b/doc/examples/avio_list_dir.c index 281b5af..4060ba6 100644 --- a/doc/examples/avio_list_dir.c +++ b/doc/examples/avio_list_dir.c @@ -74,17 +74,18 @@ int main(int argc, char *argv[]) /* register codecs and formats and other lavf/lavc components*/ av_register_all(); +avformat_network_init(); if ((ret = avio_open_dir(&ctx, input_dir, NULL)) < 0) { av_log(NULL, AV_LOG_ERROR, "Cannot open directory: %s.\n", av_err2str(ret)); -return 1; +goto fail; } cnt = 0; for (;;) { if ((ret = avio_read_dir(ctx, &entry)) < 0) { av_log(NULL, AV_LOG_ERROR, "Cannot list directory: %s.\n", av_err2str(ret)); -return 1; +goto fail; } if (!entry) break; @@ -111,7 +112,9 @@ int main(int argc, char *argv[]) cnt++; }; + fail: avio_close_dir(&ctx); +avformat_network_deinit(); -return 0; +return ret < 0 ? 1 : 0; } -- 1.9.1 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 3/3] lavf/libsmbclient: implement directory listing callbacks
On 3 April 2015 at 03:54, Mariusz Szczepańczyk wrote: > --- > libavformat/libsmbclient.c | 108 > + > 1 file changed, 108 insertions(+) > LGTM, I don't push yet as there is some discussion on other thread. I will be unavailable during following few days, so fell free to push in meanwhile. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/3] lavf/avio: Add new types to AVIODirEntryType, bump minor version
On 3 April 2015 at 16:05, Lukasz Marek wrote: > On 3 April 2015 at 15:22, Nicolas George wrote: > >> Le quartidi 14 germinal, an CCXXIII, Mariusz Szczepańczyk a écrit : >> > diff --git a/libavformat/avio.h b/libavformat/avio.h >> > index f61a8f8..51913e3 100644 >> > --- a/libavformat/avio.h >> > +++ b/libavformat/avio.h >> > @@ -63,7 +63,10 @@ enum AVIODirEntryType { >> > AVIO_ENTRY_NAMED_PIPE, >> > AVIO_ENTRY_SYMBOLIC_LINK, >> > AVIO_ENTRY_SOCKET, >> > -AVIO_ENTRY_FILE >> > +AVIO_ENTRY_FILE, >> > +AVIO_ENTRY_SERVER, >> > +AVIO_ENTRY_SHARE, >> > +AVIO_ENTRY_WORKGROUP, >> > }; >> >> Sorry if I missed the previous discussions on the mailing list (and if >> not, >> maybe just 8 hours before apply was a bit short for discussion), but I >> had a >> bit of a concern with this change. >> >> Until know, if you wanted to make a recursive listing, you just had to >> know >> that you had to recurse into directories. Now... should you recurse into >> shares? servers? workgroups? nobody knows. >> >> There should be some way of knowing whether an entry can be opened like a >> plain file, entered like a directory, or if it is just one of the weird >> things that lay in some corners of filesystems, without requiring an >> update >> when new types are added. >> > > You have good point about that, but regarding a patch itself do you see it > wrong? > Sever, share, workgroup could be mapped to directory, but what happen when > we have delete operation available in API. You can delete directory, but > how to delete a server or workgroup? > IMHO patch is OK. > > AVIODirEntry can be extended with a flag "can recurse" to indicate that, > but still I believe author of the client application must be aware of what > protocol they are using and how it works. > Simple example with samba: doc/examples/avio_list_dir smb:// will list workgroups doc/examples/avio_list_dir smb://WORKGROUP will list servers in workgroup doc/examples/avio_list_dir smb://SERVER will list shares on server doc/examples/avio_list_dir smb://SERVER/SHARE will list files, dirs etc in SHARE smb://WORKGROUP/SERVER/SHARE is invalid. So user must know how the protocol works. Samba is a bit specific case, but stacking directories, "openable" entries is not enough. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/3] lavf/avio: Add new types to AVIODirEntryType, bump minor version
On 3 April 2015 at 15:22, Nicolas George wrote: > Le quartidi 14 germinal, an CCXXIII, Mariusz Szczepańczyk a écrit : > > diff --git a/libavformat/avio.h b/libavformat/avio.h > > index f61a8f8..51913e3 100644 > > --- a/libavformat/avio.h > > +++ b/libavformat/avio.h > > @@ -63,7 +63,10 @@ enum AVIODirEntryType { > > AVIO_ENTRY_NAMED_PIPE, > > AVIO_ENTRY_SYMBOLIC_LINK, > > AVIO_ENTRY_SOCKET, > > -AVIO_ENTRY_FILE > > +AVIO_ENTRY_FILE, > > +AVIO_ENTRY_SERVER, > > +AVIO_ENTRY_SHARE, > > +AVIO_ENTRY_WORKGROUP, > > }; > > Sorry if I missed the previous discussions on the mailing list (and if not, > maybe just 8 hours before apply was a bit short for discussion), but I had > a > bit of a concern with this change. > > Until know, if you wanted to make a recursive listing, you just had to know > that you had to recurse into directories. Now... should you recurse into > shares? servers? workgroups? nobody knows. > > There should be some way of knowing whether an entry can be opened like a > plain file, entered like a directory, or if it is just one of the weird > things that lay in some corners of filesystems, without requiring an update > when new types are added. > You have good point about that, but regarding a patch itself do you see it wrong? Sever, share, workgroup could be mapped to directory, but what happen when we have delete operation available in API. You can delete directory, but how to delete a server or workgroup? IMHO patch is OK. AVIODirEntry can be extended with a flag "can recurse" to indicate that, but still I believe author of the client application must be aware of what protocol they are using and how it works. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] doc/protocols: explain FTP truncuation problem
Signed-off-by: Lukasz Marek --- doc/protocols.texi | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/doc/protocols.texi b/doc/protocols.texi index 2a19b41..73c511c 100644 --- a/doc/protocols.texi +++ b/doc/protocols.texi @@ -183,7 +183,11 @@ to be seekable. Default value is 0. NOTE: Protocol can be used as output, but it is recommended to not do it, unless special care is taken (tests, customized server configuration etc.). Different FTP servers behave in different way during seek -operation. ff* tools may produce incomplete content due to server limitations. +operation. Some servers will overwrite existing data after seeking backward, +which is expected by FFmpeg, but some servers truncate file at new position. +Unless FFmpeg can seek backward without truncuation, it may produce incomplete +or corrupted content. Some FTP servers behave in expected way by default, +some are configurable, some cannot be used to store files directly. @section gopher -- 1.9.1 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/2] lavu/dict: fix set function when reuse existing key pointer
On 02.04.2015 01:09, Michael Niedermayer wrote: On Thu, Apr 02, 2015 at 12:37:34AM +0200, Lukasz Marek wrote: On 01.04.2015 04:33, Michael Niedermayer wrote: On Wed, Apr 01, 2015 at 03:25:23AM +0200, Lukasz Marek wrote: Fixes following scenario: av_dict_set(&d, "key", "old", 0); AVDictionaryEentry *e = av_dict_get(d, "key", NULL, 0); av_dict_set(&d, e->key, "new", 0); Signed-off-by: Lukasz Marek --- libavutil/dict.c | 18 -- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/libavutil/dict.c b/libavutil/dict.c index 0d54c79..3163894 100644 --- a/libavutil/dict.c +++ b/libavutil/dict.c @@ -72,6 +72,7 @@ int av_dict_set(AVDictionary **pm, const char *key, const char *value, AVDictionary *m = *pm; AVDictionaryEntry *tag = av_dict_get(m, key, NULL, flags); char *oldval = NULL; +int the_same_key = 0; does it work to av_strdup() both key and value if they need to be strduped but at the top of the function and simplify the rest accordingly ? something like attached? yes, LGTM if tested with valgrind or similar Yes, I've tested with valgrind. Pushed. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 2/2] fate: add AVDictionary tests
On 02.04.2015 01:43, Michael Niedermayer wrote: On Thu, Apr 02, 2015 at 12:38:20AM +0200, Lukasz Marek wrote: On 01.04.2015 03:25, Lukasz Marek wrote: Signed-off-by: Lukasz Marek --- tests/fate/libavutil.mak | 4 tests/ref/fate/dict | 26 ++ I added more tests, so please see 2 attached instead LGTM Pushed both. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 2/2] fate: add AVDictionary tests
On 01.04.2015 03:25, Lukasz Marek wrote: Signed-off-by: Lukasz Marek --- tests/fate/libavutil.mak | 4 tests/ref/fate/dict | 26 ++ I added more tests, so please see 2 attached instead >From a3cca0b8533877f14ae31f2a1ecfb1e25633e536 Mon Sep 17 00:00:00 2001 From: Lukasz Marek Date: Wed, 1 Apr 2015 20:03:29 +0200 Subject: [PATCH 1/2] lavu/dict: add more tests Signed-off-by: Lukasz Marek --- libavutil/dict.c | 53 + 1 file changed, 53 insertions(+) diff --git a/libavutil/dict.c b/libavutil/dict.c index e30988d..4e4ea5f 100644 --- a/libavutil/dict.c +++ b/libavutil/dict.c @@ -276,6 +276,7 @@ static void test_separators(const AVDictionary *m, const char pair, const char v int main(void) { AVDictionary *dict = NULL; +AVDictionaryEntry *e; char *buffer = NULL; printf("Testing av_dict_get_string() and av_dict_parse_string()\n"); @@ -303,6 +304,58 @@ int main(void) test_separators(dict, '"', '\''); av_dict_free(&dict); +printf("\nTesting av_dict_set()\n"); +av_dict_set(&dict, "a", "a", 0); +av_dict_set(&dict, "b", av_strdup("b"), AV_DICT_DONT_STRDUP_VAL); +av_dict_set(&dict, av_strdup("c"), "c", AV_DICT_DONT_STRDUP_KEY); +av_dict_set(&dict, av_strdup("d"), av_strdup("d"), AV_DICT_DONT_STRDUP_KEY | AV_DICT_DONT_STRDUP_VAL); +av_dict_set(&dict, "e", "e", AV_DICT_DONT_OVERWRITE); +av_dict_set(&dict, "e", "f", AV_DICT_DONT_OVERWRITE); +av_dict_set(&dict, "f", "f", 0); +av_dict_set(&dict, "f", NULL, 0); +av_dict_set(&dict, "ff", "f", 0); +av_dict_set(&dict, "ff", "f", AV_DICT_APPEND); +e = NULL; +while ((e = av_dict_get(dict, "", e, AV_DICT_IGNORE_SUFFIX))) +printf("%s %s\n", e->key, e->value); +av_dict_free(&dict); + +av_dict_set(&dict, NULL, "a", 0); +av_dict_set(&dict, NULL, "b", 0); +av_dict_get(dict, NULL, NULL, 0); +e = NULL; +while ((e = av_dict_get(dict, "", e, AV_DICT_IGNORE_SUFFIX))) +printf("'%s' '%s'\n", e->key, e->value); +av_dict_free(&dict); + + +//valgrind sensible test +printf("\nTesting av_dict_set_int()\n"); +av_dict_set_int(&dict, "1", 1, AV_DICT_DONT_STRDUP_VAL); +av_dict_set_int(&dict, av_strdup("2"), 2, AV_DICT_DONT_STRDUP_KEY); +av_dict_set_int(&dict, av_strdup("3"), 3, AV_DICT_DONT_STRDUP_KEY | AV_DICT_DONT_STRDUP_VAL); +av_dict_set_int(&dict, "4", 4, 0); +av_dict_set_int(&dict, "5", 5, AV_DICT_DONT_OVERWRITE); +av_dict_set_int(&dict, "5", 6, AV_DICT_DONT_OVERWRITE); +av_dict_set_int(&dict, "12", 1, 0); +av_dict_set_int(&dict, "12", 2, AV_DICT_APPEND); +e = NULL; +while ((e = av_dict_get(dict, "", e, AV_DICT_IGNORE_SUFFIX))) +printf("%s %s\n", e->key, e->value); +av_dict_free(&dict); + +//valgrind sensible test +printf("\nTesting av_dict_set() with existing AVDictionaryEntry.key as key\n"); +av_dict_set(&dict, "key", "old", 0); +e = av_dict_get(dict, "key", NULL, 0); +av_dict_set(&dict, e->key, "new val OK", 0); +e = av_dict_get(dict, "key", NULL, 0); +printf("%s\n", e->value); +av_dict_set(&dict, e->key, e->value, 0); +e = av_dict_get(dict, "key", NULL, 0); +printf("%s\n", e->value); +av_dict_free(&dict); + return 0; } #endif -- 1.9.1 >From 163ff8c56de20fec3fbd32c94b557451592f3a82 Mon Sep 17 00:00:00 2001 From: Lukasz Marek Date: Wed, 1 Apr 2015 03:25:24 +0200 Subject: [PATCH 2/2] fate: add AVDictionary tests Signed-off-by: Lukasz Marek --- tests/fate/libavutil.mak | 4 tests/ref/fate/dict | 43 +++ 2 files changed, 47 insertions(+) create mode 100644 tests/ref/fate/dict diff --git a/tests/fate/libavutil.mak b/tests/fate/libavutil.mak index 58307ae..ff052e0 100644 --- a/tests/fate/libavutil.mak +++ b/tests/fate/libavutil.mak @@ -53,6 +53,10 @@ fate-des: libavutil/des-test$(EXESUF) fate-des: CMD = run libavutil/des-test fate-des: REF = /dev/null +FATE_LIBAVUTIL += fate-dict +fate-dict: libavutil/dict-test$(EXESUF) +fate-dict: CMD = run libavutil/dict-test + FATE_LIBAVUTIL += fate-eval fate-eval: libavutil/eval-test$(EXESUF) fate-eval: CMD = run libavutil/eval-test diff --git a/tests/ref/fate/dict b/tests/ref/fate/dict new file mode 1
Re: [FFmpeg-devel] [PATCH 1/2] lavu/dict: fix set function when reuse existing key pointer
On 01.04.2015 04:33, Michael Niedermayer wrote: On Wed, Apr 01, 2015 at 03:25:23AM +0200, Lukasz Marek wrote: Fixes following scenario: av_dict_set(&d, "key", "old", 0); AVDictionaryEentry *e = av_dict_get(d, "key", NULL, 0); av_dict_set(&d, e->key, "new", 0); Signed-off-by: Lukasz Marek --- libavutil/dict.c | 18 -- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/libavutil/dict.c b/libavutil/dict.c index 0d54c79..3163894 100644 --- a/libavutil/dict.c +++ b/libavutil/dict.c @@ -72,6 +72,7 @@ int av_dict_set(AVDictionary **pm, const char *key, const char *value, AVDictionary *m = *pm; AVDictionaryEntry *tag = av_dict_get(m, key, NULL, flags); char *oldval = NULL; +int the_same_key = 0; does it work to av_strdup() both key and value if they need to be strduped but at the top of the function and simplify the rest accordingly ? something like attached? >From 2a36567b12ae14798306a988061fe30474c5c332 Mon Sep 17 00:00:00 2001 From: Lukasz Marek Date: Wed, 1 Apr 2015 20:01:30 +0200 Subject: [PATCH] lavu/dict: fix set function when reuse existing key pointer Fixes following scenario: av_dict_set(&d, "key", "old", 0); AVDictionaryEentry *e = av_dict_get(d, "key", NULL, 0); av_dict_set(&d, e->key, "new", 0); Signed-off-by: Lukasz Marek --- libavutil/dict.c | 44 1 file changed, 24 insertions(+), 20 deletions(-) diff --git a/libavutil/dict.c b/libavutil/dict.c index 0d54c79..1dfcb68 100644 --- a/libavutil/dict.c +++ b/libavutil/dict.c @@ -71,17 +71,25 @@ int av_dict_set(AVDictionary **pm, const char *key, const char *value, { AVDictionary *m = *pm; AVDictionaryEntry *tag = av_dict_get(m, key, NULL, flags); -char *oldval = NULL; +char *oldval = NULL, *copy_key = NULL, *copy_value = NULL; +if (flags & AV_DICT_DONT_STRDUP_KEY) +copy_key = (void *)key; +else +copy_key = av_strdup(key); +if (flags & AV_DICT_DONT_STRDUP_VAL) +copy_value = (void *)value; +else if (copy_key) +copy_value = av_strdup(value); if (!m) m = *pm = av_mallocz(sizeof(*m)); -if (!m) +if (!m || (key && !copy_key) || (value && !copy_value)) goto err_out; if (tag) { if (flags & AV_DICT_DONT_OVERWRITE) { -if (flags & AV_DICT_DONT_STRDUP_KEY) av_free((void*)key); -if (flags & AV_DICT_DONT_STRDUP_VAL) av_free((void*)value); +av_free(copy_key); +av_free(copy_value); return 0; } if (flags & AV_DICT_APPEND) @@ -97,27 +105,23 @@ int av_dict_set(AVDictionary **pm, const char *key, const char *value, goto err_out; m->elems = tmp; } -if (value) { -if (flags & AV_DICT_DONT_STRDUP_KEY) -m->elems[m->count].key = (char*)(intptr_t)key; -else -m->elems[m->count].key = av_strdup(key); -if (!m->elems[m->count].key) -goto err_out; -if (flags & AV_DICT_DONT_STRDUP_VAL) { -m->elems[m->count].value = (char*)(intptr_t)value; -} else if (oldval && flags & AV_DICT_APPEND) { -int len = strlen(oldval) + strlen(value) + 1; +if (copy_value) { +m->elems[m->count].key = copy_key; +m->elems[m->count].value = copy_value; +if (oldval && flags & AV_DICT_APPEND) { +int len = strlen(oldval) + strlen(copy_value) + 1; char *newval = av_mallocz(len); if (!newval) goto err_out; av_strlcat(newval, oldval, len); av_freep(&oldval); -av_strlcat(newval, value, len); +av_strlcat(newval, copy_value, len); m->elems[m->count].value = newval; -} else -m->elems[m->count].value = av_strdup(value); +av_freep(©_value); +} m->count++; +} else { +av_freep(©_key); } if (!m->count) { av_freep(&m->elems); @@ -131,8 +135,8 @@ err_out: av_freep(&m->elems); av_freep(pm); } -if (flags & AV_DICT_DONT_STRDUP_KEY) av_free((void*)key); -if (flags & AV_DICT_DONT_STRDUP_VAL) av_free((void*)value); +av_free(copy_key); +av_free(copy_value); return AVERROR(ENOMEM); } -- 1.9.1 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] lavu/dict: don't accept AV_DICT_DONT_STRDUP_VAL for av_dict_set_int
Signed-off-by: Lukasz Marek --- libavutil/dict.c | 1 + 1 file changed, 1 insertion(+) diff --git a/libavutil/dict.c b/libavutil/dict.c index 1dfcb68..e30988d 100644 --- a/libavutil/dict.c +++ b/libavutil/dict.c @@ -145,6 +145,7 @@ int av_dict_set_int(AVDictionary **pm, const char *key, int64_t value, { char valuestr[22]; snprintf(valuestr, sizeof(valuestr), "%"PRId64, value); +flags &= ~AV_DICT_DONT_STRDUP_VAL; return av_dict_set(pm, key, valuestr, flags); } -- 1.9.1 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] lavf: add zip protocol
On 01.04.2015 21:22, Alexander Strasser wrote: On 2015-03-28 23:30 +0100, Lukasz Marek wrote: W dniu sobota, 28 marca 2015 Peter Ross napisał(a): What about the following? ffplay zip://dir/a.zip/m.mkv # open dir/a.zip, read file m.mkv inside ffplay -file m.mkv zip://dir/a.zip # same effect as above ffplay zip://dir.zip/a.ass/m.txt # open dir.zip, read file a.ass/m.txt inside ffplay -file m.txt zip://dir.zip/a.ass # open dir.zip/a.ass, read file m.txt inside Patch that implements that on top of Lukasz's patch is attached. Beware it is quickly implemented and only lightly tested, just to show that it is possible to implement that behaviour. Do not expect it to be beautiful and flawless. Also I didn't even attempt to work out details like option name and if we should match first or last .zip occurrence when guessing. I've attached another implementation based on libarchive. It is also just demo patch, not something that is ready to be merged. I skipped totally the problem with selecting internal file. I just open the first available. Libarchive allows: 1. Using protocols, so archive can be on remote host, tested via ftp and sftp (didn't check extracting from inner archive) 2. It provides archive_seek_data which seems to allow make this protocol seekable, but it is not working for me. Official documentation doesn't mention this function. Doxy in header is not much more helpful. Libarchive does not allows: 1. Decompress encrypted archives (there is a ticket for that) 2. I cannot decompress rar archives (still better than libzip) It still needs some tests and possibly digging in libarchive code. Some contact with them may resolve some doubts. Regarding selecting inner file, IMHO additional parameter and guessing by extension is the best choice until protocols support stat operation in efficient way. Of course assuming it should use protocols. I will not work on this withing next week at least so if someone wants to continue then go ahead, just let me know please, so we don't duplicate the work again. >From 71e69b5d3beed3429a310c413e4139f640e837e2 Mon Sep 17 00:00:00 2001 From: Lukasz Marek Date: Fri, 20 Mar 2015 18:31:00 +0100 Subject: [PATCH] lavf: add zip protocol TODO: add doc, update doc/APIChanges, bump minor --- configure| 4 + libavformat/Makefile | 2 + libavformat/allformats.c | 1 + libavformat/libarchive.c | 193 +++ 4 files changed, 200 insertions(+) create mode 100644 libavformat/libarchive.c diff --git a/configure b/configure index 392946a..9cb4cfc 100755 --- a/configure +++ b/configure @@ -276,6 +276,7 @@ External library support: --disable-sdldisable sdl [autodetect] --enable-x11grab enable X11 grabbing (legacy) [no] --disable-xlib disable xlib [autodetect] + --enable-libarchive enable libarchive [no] --disable-zlib disable zlib [autodetect] Toolchain options: @@ -1406,6 +1407,7 @@ EXTERNAL_LIBRARY_LIST=" libxcb_shape libxcb_xfixes libxvid +libarchive libzmq libzvbi lzma @@ -2599,6 +2601,7 @@ udp_protocol_select="network" udplite_protocol_select="network" unix_protocol_deps="sys_un_h" unix_protocol_select="network" +libarchive_protocol_deps="libarchive" # filters amovie_filter_deps="avcodec avformat" @@ -4981,6 +4984,7 @@ enabled libsmbclient && { use_pkg_config smbclient libsmbclient.h smbc_init require smbclient libsmbclient.h smbc_init -lsmbclient; } enabled libsoxr && require libsoxr soxr.h soxr_create -lsoxr enabled libssh&& require_pkg_config libssh libssh/sftp.h sftp_init +enabled libarchive&& require_pkg_config libarchive archive.h archive_read_open enabled libspeex && require_pkg_config speex speex/speex.h speex_decoder_init -lspeex enabled libstagefright_h264 && require_cpp libstagefright_h264 "binder/ProcessState.h media/stagefright/MetaData.h media/stagefright/MediaBufferGroup.h media/stagefright/MediaDebug.h media/stagefright/MediaDefs.h diff --git a/libavformat/Makefile b/libavformat/Makefile index 2118ff2..c971ef1 100644 --- a/libavformat/Makefile +++ b/libavformat/Makefile @@ -522,6 +522,8 @@ OBJS-$(CONFIG_TLS_PROTOCOL) += tls.o OBJS-$(CONFIG_UDP_PROTOCOL) += udp.o OBJS-$(CONFIG_UDPLITE_PROTOCOL) += udp.o OBJS-$(CONFIG_UNIX_PROTOCOL) += unix.o +OBJS-$(CONFIG_LIBARCHIVE_PROTOCOL) += libarchive.o + OBJS-$(HAVE_LIBC_MSVCRT) += file_open.o diff --git a/libavformat/allformats.c b/libavformat/allformats.c index 26ccc27..9a9eef4 100644 --- a/libavformat/allformats.c +++ b/libavformat/allformats.c @@ -379,6 +379,7 @@ void av_register_al
[FFmpeg-devel] [PATCH 2/2] fate: add AVDictionary tests
Signed-off-by: Lukasz Marek --- tests/fate/libavutil.mak | 4 tests/ref/fate/dict | 26 ++ 2 files changed, 30 insertions(+) create mode 100644 tests/ref/fate/dict diff --git a/tests/fate/libavutil.mak b/tests/fate/libavutil.mak index 58307ae..ff052e0 100644 --- a/tests/fate/libavutil.mak +++ b/tests/fate/libavutil.mak @@ -53,6 +53,10 @@ fate-des: libavutil/des-test$(EXESUF) fate-des: CMD = run libavutil/des-test fate-des: REF = /dev/null +FATE_LIBAVUTIL += fate-dict +fate-dict: libavutil/dict-test$(EXESUF) +fate-dict: CMD = run libavutil/dict-test + FATE_LIBAVUTIL += fate-eval fate-eval: libavutil/eval-test$(EXESUF) fate-eval: CMD = run libavutil/eval-test diff --git a/tests/ref/fate/dict b/tests/ref/fate/dict new file mode 100644 index 000..deb61ae --- /dev/null +++ b/tests/ref/fate/dict @@ -0,0 +1,26 @@ +Testing av_dict_get_string() and av_dict_parse_string() + +aaa aaa b,b bbb c=c ccc ddd d,d eee e=e f,f f=f g=g g,g +aaa=aaa,b\,b=bbb,c\=c=ccc,ddd=d\,d,eee=e\=e,f\,f=f\=f,g\=g=g\,g +aaa aaa b,b bbb c=c ccc ddd d,d eee e=e f,f f=f g=g g,g +aaa aaa bbb bbb ccc ccc \,='" \,='" +aaa=aaa"bbb=bbb"ccc=ccc"\\,\=\'\"=\\,\=\'\" +aaa aaa bbb bbb ccc ccc \,='" \,='" +aaa aaa bbb bbb ccc ccc \,='" \,='" +aaa=aaa'bbb=bbb'ccc=ccc'\\,\=\'"=\\,\=\'" +aaa aaa bbb bbb ccc ccc \,='" \,='" +aaa aaa bbb bbb ccc ccc \,='" \,='" +aaa"aaa,bbb"bbb,ccc"ccc,\\\,=\'\""\\\,=\'\" +aaa aaa bbb bbb ccc ccc \,='" \,='" +aaa aaa bbb bbb ccc ccc \,='" \,='" +aaa'aaa,bbb'bbb,ccc'ccc,\\\,=\'"'\\\,=\'" +aaa aaa bbb bbb ccc ccc \,='" \,='" +aaa aaa bbb bbb ccc ccc \,='" \,='" +aaa"aaa'bbb"bbb'ccc"ccc'\\,=\'\""\\,=\'\" +aaa aaa bbb bbb ccc ccc \,='" \,='" +aaa aaa bbb bbb ccc ccc \,='" \,='" +aaa'aaa"bbb'bbb"ccc'ccc"\\,=\'\"'\\,=\'\" +aaa aaa bbb bbb ccc ccc \,='" \,='" + +Testing av_dict_set() with existing AVDictionaryEntry.key as key +new val OK -- 1.9.1 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 1/2] lavu/dict: fix set function when reuse existing key pointer
Fixes following scenario: av_dict_set(&d, "key", "old", 0); AVDictionaryEentry *e = av_dict_get(d, "key", NULL, 0); av_dict_set(&d, e->key, "new", 0); Signed-off-by: Lukasz Marek --- libavutil/dict.c | 18 -- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/libavutil/dict.c b/libavutil/dict.c index 0d54c79..3163894 100644 --- a/libavutil/dict.c +++ b/libavutil/dict.c @@ -72,6 +72,7 @@ int av_dict_set(AVDictionary **pm, const char *key, const char *value, AVDictionary *m = *pm; AVDictionaryEntry *tag = av_dict_get(m, key, NULL, flags); char *oldval = NULL; +int the_same_key = 0; if (!m) m = *pm = av_mallocz(sizeof(*m)); @@ -88,7 +89,10 @@ int av_dict_set(AVDictionary **pm, const char *key, const char *value, oldval = tag->value; else av_free(tag->value); -av_free(tag->key); +if (tag->key != key) +av_free(tag->key); +else +the_same_key = 1; *tag = m->elems[--m->count]; } else { AVDictionaryEntry *tmp = av_realloc(m->elems, @@ -98,7 +102,7 @@ int av_dict_set(AVDictionary **pm, const char *key, const char *value, m->elems = tmp; } if (value) { -if (flags & AV_DICT_DONT_STRDUP_KEY) +if (flags & AV_DICT_DONT_STRDUP_KEY || the_same_key) m->elems[m->count].key = (char*)(intptr_t)key; else m->elems[m->count].key = av_strdup(key); @@ -271,6 +275,7 @@ static void test_separators(const AVDictionary *m, const char pair, const char v int main(void) { AVDictionary *dict = NULL; +AVDictionaryEntry *e; char *buffer = NULL; printf("Testing av_dict_get_string() and av_dict_parse_string()\n"); @@ -298,6 +303,15 @@ int main(void) test_separators(dict, '"', '\''); av_dict_free(&dict); +//valgrind sensible test +printf("\nTesting av_dict_set() with existing AVDictionaryEntry.key as key\n"); +av_dict_set(&dict, "key", "old", 0); +e = av_dict_get(dict, "key", NULL, 0); +av_dict_set(&dict, e->key, "new val OK", 0); +e = av_dict_get(dict, "key", NULL, 0); +printf("%s\n", e->value); +av_dict_free(&dict); + return 0; } #endif -- 1.9.1 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] lavf: Add support for WebM Live Muxing
When are we going to get a DASH demuxer? Why don't you implement it? ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/4] lavf: add directory listing API
On 30.03.2015 03:01, Michael Niedermayer wrote: On Mon, Mar 30, 2015 at 12:36:34AM +0200, Lukasz Marek wrote: On 29.03.2015 01:14, Mariusz Szczepańczyk wrote: diff --git a/doc/APIchanges b/doc/APIchanges index 3f153e9..814f752 100644 --- a/doc/APIchanges +++ b/doc/APIchanges @@ -15,6 +15,15 @@ libavutil: 2014-08-09 API changes, most recent first: +2015-03-27 - 184084c - lavf 56.27.100 - avio.h url.h + New directory listing API. + + Add AVIODirEntryType enum. + Add AVIODirEntry, AVIODirContext structures. + Add avio_open_dir(), avio_read_dir(), avio_close_dir(), avio_free_directory_entry(). + Add ff_alloc_dir_entry(). + Extend URLProtocol with url_open_dir(), url_read_dir(), url_close_dir(). It can be simple "add url_open_dir()...", but it is OK I think + 8< - FFmpeg 2.6 was cut here 8< - 2015-03-04 - cca4476 - lavf 56.25.100 diff --git a/libavformat/version.h b/libavformat/version.h index a183d7f..ff85227 100644 --- a/libavformat/version.h +++ b/libavformat/version.h @@ -30,7 +30,7 @@ #include "libavutil/version.h" #define LIBAVFORMAT_VERSION_MAJOR 56 -#define LIBAVFORMAT_VERSION_MINOR 26 +#define LIBAVFORMAT_VERSION_MINOR 27 Michaels, do you gonna merge it? this one still had a minor issue in the version number but if the next looks fine to you or you want to correct it yourself dont hesitate to apply it Fixed locally and pushed. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/4] lavf: add directory listing API
On 29.03.2015 01:14, Mariusz Szczepańczyk wrote: diff --git a/doc/APIchanges b/doc/APIchanges index 3f153e9..814f752 100644 --- a/doc/APIchanges +++ b/doc/APIchanges @@ -15,6 +15,15 @@ libavutil: 2014-08-09 API changes, most recent first: +2015-03-27 - 184084c - lavf 56.27.100 - avio.h url.h + New directory listing API. + + Add AVIODirEntryType enum. + Add AVIODirEntry, AVIODirContext structures. + Add avio_open_dir(), avio_read_dir(), avio_close_dir(), avio_free_directory_entry(). + Add ff_alloc_dir_entry(). + Extend URLProtocol with url_open_dir(), url_read_dir(), url_close_dir(). It can be simple "add url_open_dir()...", but it is OK I think + 8< - FFmpeg 2.6 was cut here 8< - 2015-03-04 - cca4476 - lavf 56.25.100 diff --git a/libavformat/version.h b/libavformat/version.h index a183d7f..ff85227 100644 --- a/libavformat/version.h +++ b/libavformat/version.h @@ -30,7 +30,7 @@ #include "libavutil/version.h" #define LIBAVFORMAT_VERSION_MAJOR 56 -#define LIBAVFORMAT_VERSION_MINOR 26 +#define LIBAVFORMAT_VERSION_MINOR 27 Michaels, do you gonna merge it? ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] lavf: add zip protocol
W dniu sobota, 28 marca 2015 Peter Ross napisał(a): > On Sat, Mar 28, 2015 at 08:38:40PM +0100, Lukasz Marek wrote: > > I assumed it is local file (no other option so far). So I stat full path > > (/tmp/outer.zip/tmp/inner.zip/tmp/data.bin) for being a file, if so then > I > > opened it as zip file and used fallback to open first file. > > If not then I stat by path components: /tmp/, /tmp/outer.zip, and so > on... > > /tmp/outer.zip is a file so I open it and treat rest of the uri as a path > > inside zip. > > walking the path means that the archive protocol must know about the > syntax of the underlying protocol (file, http, ftp, etc.). that seems > messy. > also inefficient if you have got to walk a long ftp path. I think filesystem looks the same on every server, no matter it is http, ftp or any other server. wouldn't we be better off defining a special character that seperates the > zip > path from the inner path. obviously we'd need some way of escaping the > character > if it is legitimately part of either path. In real life you might assume that archive has extension zip, rar, 7z,... and try to open according to it in first try, if user has archive with "ass" extension in directory called "data.zip" then they have to wait. i really would avoid to escape anything. this is just annoying. i remember i used to escape urls with so many ifs, if file then dont escape, if http then escape, and so on. handling archives with its own escapes is kinda stupid. it would be easier just to add an option to pass inner file. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] lavf: add zip protocol
On 28.03.2015 20:13, Nicolas George wrote: L'octidi 8 germinal, an CCXXIII, Lukasz Marek a écrit : I will try to use this libarchive first and do some tests. Your approach may collapse in case compression libraries doesn't support parallel compression/decompression (I mean that you write or read several files from single archive file) I would be much surprised if at least writing will not work. This is a likely issue, but fortunately it would not prevent all use cases. I wonder if there is other solution: zip could be protocol as it is now, it allows to benefit from list API and gives flexibility other demuxers to benefit from it. There could also be a "directory" demuxer which would also use that API and possibly could serve streams in your way. That demuxer could also handle directories over any protocol that supports that API. That was the kind of idea that I had. But I believe that to get that working a bit reliably, we will need to extend the directory listing callbacks to allow a URL context to create new URL contexts, to open remote files without establishing a new connection (and it will also be necessary for network servers). Some kind of VFS API, then. This can be even harder as opening archive file require stat or other smart way to check some candidates that ought to be a archive file. See below. ffplay zip:///tmp/outer.zip/tmp/inner.zip/tmp/data.bin libzip can't handle it (the same way it cannot handle files via protocols), maybe libarchive will be better I think you misunderstood the question. I was not asking whether it would be able to decode nested files, but how your code did split nested paths: would it try to find /tmp/inner.zip/tmp/data.bin inside /tmp/outer.zip, or /tmp/data.bin inside /tmp/outer.zip/tmp/inner.zip (assuming someone was stupid enough to name a directory dot-zip)? I assumed it is local file (no other option so far). So I stat full path (/tmp/outer.zip/tmp/inner.zip/tmp/data.bin) for being a file, if so then I opened it as zip file and used fallback to open first file. If not then I stat by path components: /tmp/, /tmp/outer.zip, and so on... /tmp/outer.zip is a file so I open it and treat rest of the uri as a path inside zip. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] lavf: add zip protocol
On 28.03.2015 11:53, Nicolas George wrote: Le septidi 7 germinal, an CCXXIII, Lukasz Marek a écrit : But, this time I dont understand you comments, could you elaborate it? What's wrong, what can I do? What I am saying is that there are a lot of different cases where we want to read archives (not only zip, see my previous mail, but that does not matter for now) on the fly with FFmpeg, and I am not sure your proposal is convenient for all these use cases. You implement it as a protocol, to access a single file in the archive. If it was gzip, that would be fine, because it is a stream compression format. In fact, I now realize we should have stackable protocols for all common stream compression tools. But a zip file is not just a compressed stream: it contains structure, several files. In FFmpeg architecture, that may map better to a demuxer: each file as a stream or as an attachment. This is yet another case where the distinction between protocols and formats is not entirely clear. If you think about it, an input protocol is just a demuxer that outputs a single stream of AVMEDIA_TYPE_DATA packets. Of course, for "normal" protocols and formats, like reading a Matroska file from a plain file, the separation makes sense. But with more complex and tied-in protocols and formats, it makes things actually harder. See the RTP issues for example. I have not yet looked closely enough at it, but I suspect the directory listing API that you have just landed may start a bridge between the two: a protocol may no longer be just an API for accessing a single stream but a whole filesystem. Then we can have demuxers that use it. I suppose one of the most pressing tasks would be to have the image2 demuxer use the directory listing API, is it not? I know that line between protocol and format is very thin. I will try to use this libarchive first and do some tests. Your approach may collapse in case compression libraries doesn't support parallel compression/decompression (I mean that you write or read several files from single archive file) I would be much surprised if at least writing will not work. But I will test it, there is no point in guessing here. Of course making it protocol doesn't solve that potential issue, but it may be less confusing for the user. I wonder if there is other solution: zip could be protocol as it is now, it allows to benefit from list API and gives flexibility other demuxers to benefit from it. There could also be a "directory" demuxer which would also use that API and possibly could serve streams in your way. That demuxer could also handle directories over any protocol that supports that API. Personally I don't favor any of the approaches, but if I had to decide then probably a protocol. So my actual proposal about this patch is: keep it near at hand, but not apply it; rather, use it as a work case to see the most we can do with new APIs. (Well, I do not oppose actually applying it. But if so, let us make us very clear that this is something really experimental. Not experimental "it probably works poorly" but experimental "we may change it completely tomorrow because we had another idea, we will not bother AT ALL with compatibility for now".) I have no pressure to merge asap. At least this libarchive is worth to try. I think you misunderstood this. There is no doc, but reading files by index is a fallback when user doesn't specify file explicitly. For example: Thanks for correcting me, I really missed that. ./ffplay zip://zipfile.zip/aaa.avi Ok, but that leads me to another question: what does this do: ffplay zip:///tmp/outer.zip/tmp/inner.zip/tmp/data.bin libzip can't handle it (the same way it cannot handle files via protocols), maybe libarchive will be better ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 4/4] lavf/libsmbclient: implement directory listing callbacks
On 26.03.2015 01:25, Mariusz Szczepańczyk wrote: --- libavformat/libsmbclient.c | 93 ++ 1 file changed, 93 insertions(+) [...] +switch (dirent->smbc_type) { +case SMBC_DIR: +entry->type = AVIO_ENTRY_DIRECTORY; +break; +case SMBC_FILE: +entry->type = AVIO_ENTRY_FILE; +break; +default: +/* TODO: Find out what other types stand for and their correct + * mappings. Probably some of them should be skipped. */ +entry->type = AVIO_ENTRY_UNKNOWN; +break; +} Before it is merged it should handle: SMBC_WORKGROUP, SMBC_SERVER, SMBC_FILE_SHARE This implicates that AVIODirEntryType has to be extended and doc/examples/avio_list_dir.c from second patch as well, It im not sure how to handle SMBC_PRINTER_SHARE, SMBC_COMMS_SHARE, SMBC_IPC_SHARE It can be set as unknown, it can be not listed at all, or also specific enums can be added. Printers are not relevant to ffmpeg at all. Patch itself is ok, but it is not complete. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] lavf: add zip protocol
On 27.03.2015 15:49, Nicolas George wrote: Le septidi 7 germinal, an CCXXIII, Lukasz Marek a écrit : TODO: add doc, update doc/APIChanges, bump minor This is interesting, but the use case must be considered very carefully, there are tons of things that can be done with a zip file, even relevant to FFmpeg: - demux a bunch of images as a kind of image2 stream; - present an archive of font files as attachments (with the patch I have not yet pushed to MPlayer, that means you could use them directly); - read several files in parallel (with split audio and video for example; does the zip format allow interleaving); - etc. Hi Nicolas, I missed you comment my patches :) But, this time I dont understand you comments, could you elaborate it? What's wrong, what can I do? +{"file_index", "set file index", OFFSET(file_index), AV_OPT_TYPE_INT, {.i64 = 0}, -1, INT_MAX, D|E }, That is not very user-friendly. I think you misunderstood this. There is no doc, but reading files by index is a fallback when user doesn't specify file explicitly. For example: ./ffplay zip://zipfile.zip/aaa.avi will provide aaa.avi file from zipfile.zip ./ffplay zip://zipfile.zip will provide first file (with index 0) in zip file ./ffplay -file_index n zip://zipfile.zip will provide n + 1 (with index n) file in zip file this might be helpful when user know there is only one file in zip archive. Yes, it must be documented. Probably it can be even better to allow provide whitelist/blacklist for filenames. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH/TOY] zip files
On 27.03.2015 14:16, Michael Niedermayer wrote: On Fri, Mar 27, 2015 at 01:51:08PM +0100, Lukasz Marek wrote: On 27.03.2015 13:07, Peter Ross wrote: --- this was created to test the idea proposed in https://trac.ffmpeg.org/ticket/4374 as result, i think a zip:// protocol has some problems - strict uri syntax can only open files in current directory - user shouldn't have to specify the archive format. this should be autodetected (zip, rar, ... arj) - want ability to grab zip files over other protocols 'vfs:http://subtitles.org/amovie.zip:amovie.srt' - default action to open the first inner file? We duplicated out work. I implemented it too, just wanted to add write too and have no time recently. I submitted my version in separate thread. Issue 1 and 4 are supported in my patch. Issue 2 is not the issue, it just need to be implemented, but in this case this "protocol" should be called "archive" or something. Regarding Issue 3 this is probably not possible with libzip. It allows to open archive by descriptor, so it gives possibility to get http's or ftp's handle in theory (or ffmpeg could created named pipe and push data from remote host into it), but I expect this need to be seekable. I haven't tested yet though. if libzip cant be used, is there something else that can be used ? or can libzip be extended to not have this limitation ? Just tested descriptors, I tried to open named pipe, like mkfifo test.zip cat data.zip > test.zip ./ffplay zip://test.zip and libzip seg faulted :) Reverted order mkfifo test.zip ./ffplay zip://test.zip cat data.zip > test.zip postpones seg fault until cat is executed. So this will not work. libzip probably could be extended by open function that calls a callback to get data, like: int(cb*)(size_t offset, void* buff, size_t size), I haven't checked its sources, but they have similar API for write. In real life it will take ages that API propagates into linux distributions. Someone who really needs this can always build it on its own tho. I haven't researched deeply if there are other libs for handling zips. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] lavf: add zip protocol
TODO: add doc, update doc/APIChanges, bump minor --- configure| 4 ++ libavformat/Makefile | 2 + libavformat/allformats.c | 1 + libavformat/zip.c| 176 +++ 4 files changed, 183 insertions(+) create mode 100644 libavformat/zip.c diff --git a/configure b/configure index a96bfb7..a36c357 100755 --- a/configure +++ b/configure @@ -275,6 +275,7 @@ External library support: --disable-sdldisable sdl [autodetect] --enable-x11grab enable X11 grabbing (legacy) [no] --disable-xlib disable xlib [autodetect] + --enable-libzip enable libzip [no] --disable-zlib disable zlib [autodetect] Toolchain options: @@ -1404,6 +1405,7 @@ EXTERNAL_LIBRARY_LIST=" libxcb_shape libxcb_xfixes libxvid +libzip libzmq libzvbi lzma @@ -2582,6 +2584,7 @@ udp_protocol_select="network" udplite_protocol_select="network" unix_protocol_deps="sys_un_h" unix_protocol_select="network" +zip_protocol_deps="zlib" # filters amovie_filter_deps="avcodec avformat" @@ -4963,6 +4966,7 @@ enabled libsmbclient && { use_pkg_config smbclient libsmbclient.h smbc_init require smbclient libsmbclient.h smbc_init -lsmbclient; } enabled libsoxr && require libsoxr soxr.h soxr_create -lsoxr enabled libssh&& require_pkg_config libssh libssh/sftp.h sftp_init +enabled libzip&& require_pkg_config libzip zip.h zip_open enabled libspeex && require_pkg_config speex speex/speex.h speex_decoder_init -lspeex enabled libstagefright_h264 && require_cpp libstagefright_h264 "binder/ProcessState.h media/stagefright/MetaData.h media/stagefright/MediaBufferGroup.h media/stagefright/MediaDebug.h media/stagefright/MediaDefs.h diff --git a/libavformat/Makefile b/libavformat/Makefile index 2118ff2..47d3804 100644 --- a/libavformat/Makefile +++ b/libavformat/Makefile @@ -522,6 +522,8 @@ OBJS-$(CONFIG_TLS_PROTOCOL) += tls.o OBJS-$(CONFIG_UDP_PROTOCOL) += udp.o OBJS-$(CONFIG_UDPLITE_PROTOCOL) += udp.o OBJS-$(CONFIG_UNIX_PROTOCOL) += unix.o +OBJS-$(CONFIG_ZIP_PROTOCOL) += zip.o + OBJS-$(HAVE_LIBC_MSVCRT) += file_open.o diff --git a/libavformat/allformats.c b/libavformat/allformats.c index 26ccc27..96fe484 100644 --- a/libavformat/allformats.c +++ b/libavformat/allformats.c @@ -379,6 +379,7 @@ void av_register_all(void) REGISTER_PROTOCOL(UDP, udp); REGISTER_PROTOCOL(UDPLITE, udplite); REGISTER_PROTOCOL(UNIX, unix); +REGISTER_PROTOCOL(ZIP, zip); /* external libraries */ REGISTER_DEMUXER (LIBGME, libgme); diff --git a/libavformat/zip.c b/libavformat/zip.c new file mode 100644 index 000..c9d6679 --- /dev/null +++ b/libavformat/zip.c @@ -0,0 +1,176 @@ +/* + * Copyright (c) 2015 Lukasz Marek + * + * This file is part of FFmpeg. + * + * FFmpeg is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * FFmpeg is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with FFmpeg; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + */ + +#include +#include +#include "libavutil/opt.h" +#include "libavutil/avstring.h" +#include "avformat.h" +#include "internal.h" +#include "url.h" + +typedef struct { +const AVClass *class; + +struct zip *z; +struct zip_file *f; + +char *password; +int file_index; +int64_t filesize; +} ZIPContext; + + +static int ffzip_close(URLContext *h) +{ +ZIPContext *zip = h->priv_data; +if (zip->f) { +zip_fclose(zip->f); +zip->f = NULL; +} +if (zip->z) { +zip_close(zip->z); +zip->z = NULL; +} +return 0; +} + +static int ffzip_split_url(URLContext *h, const char *url, char **file) +{ +ZIPContext *zip = h->priv_data; +int ret = 0; +struct stat st; +char *filename, *pos; + +*file = NULL; + +if (strnlen(url, 7) < 7 || !av_strstart(url, "zip://", NULL)) +return AVERROR(EINVAL); + +filename = av_strdup(url + 6); +if (!filename) +return AVERROR(ENOMEM); + +pos = filename; +wh
Re: [FFmpeg-devel] [PATCH/TOY] zip files
On 27.03.2015 13:07, Peter Ross wrote: --- this was created to test the idea proposed in https://trac.ffmpeg.org/ticket/4374 as result, i think a zip:// protocol has some problems - strict uri syntax can only open files in current directory - user shouldn't have to specify the archive format. this should be autodetected (zip, rar, ... arj) - want ability to grab zip files over other protocols 'vfs:http://subtitles.org/amovie.zip:amovie.srt' - default action to open the first inner file? We duplicated out work. I implemented it too, just wanted to add write too and have no time recently. I submitted my version in separate thread. Issue 1 and 4 are supported in my patch. Issue 2 is not the issue, it just need to be implemented, but in this case this "protocol" should be called "archive" or something. Regarding Issue 3 this is probably not possible with libzip. It allows to open archive by descriptor, so it gives possibility to get http's or ftp's handle in theory (or ffmpeg could created named pipe and push data from remote host into it), but I expect this need to be seekable. I haven't tested yet though. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] GSoC project proposal + qualification task
On 21.03.2015 19:01, wm4 wrote: On Fri, 20 Mar 2015 02:51:31 +0100 Mariusz Szczepańczyk wrote: Hello, my name is Mariusz Szczepańczyk and I am currently finishing my bachelor's degree in Computer Science at the University of Warsaw in Poland. I have written few patches for other open source projects, like gif reading plugin for OpenImageIO, however I'm new to FFmpeg. I'd like to work on the "Browsing content on the server" task as described on wiki extended by samba and possibly local zip archives if it gets a good reception. And maybe having rename and delete methods also could be nice. I reached out to Lukasz Marek, who is listed as backup mentor and he pointed me to his last year's patches that add directory listing api and implement some of the protocols. In a couple of minutes I'll send these patches rebased against current master plus my take on samba protocol. Any comments and ideas are appreciated. If the patches you posted (and which are not even your work, just rebased older patches from someone else) are the qualification task, then what's left for the "real" project work? Qualification task it to implement one protocol using provided API, so I don't understand your complaining as this requirement is met. "Real" project work is to implement this feature for HTTP (which is not standardized as far is i know) and for FTP LIST command fallback (which is not standardized neither), and handle some rare cases I surly didn't included in my patches. Also you may refer my previous mails about extending this project as some *easy* work is already done. Just comment from me; for people who participate in ffmpeg for some time, your comment is just like your comments are (rude, demotivating, and ofter useless), but for students who supposed to have some help and support here, your comment is just stupid. If you think this project is stupid, then you should've removed it from the project list before someone involved into it. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] GSoC project proposal + qualification task
On 20.03.2015 02:51, Mariusz Szczepańczyk wrote: Hello, my name is Mariusz Szczepańczyk and I am currently finishing my bachelor's degree in Computer Science at the University of Warsaw in Poland. I have written few patches for other open source projects, like gif reading plugin for OpenImageIO, however I'm new to FFmpeg. I'd like to work on the "Browsing content on the server" task as described on wiki extended by samba and possibly local zip archives if it gets a good reception. And maybe having rename and delete methods also could be nice. I reached out to Lukasz Marek, who is listed as backup mentor and he pointed me to his last year's patches that add directory listing api and implement some of the protocols. In a couple of minutes I'll send these patches rebased against current master plus my take on samba protocol. Any comments and ideas are appreciated. Hi, Thanks for a submit. To clarify, a qualification task is to implement this feature for one protocol, and samba's implementation seems correct (not tested yet though). Patches you resubmitted for libssh, file ftp have to be reviewed and fixed too, but this may be done later, not necessarily during qualification task submission. As you see, some of the last year project's job is already done, so this project may be extended by implementing delete and rename API as well. It has about the same complexity as the listing API that is already implemented. I will add this task to list and set myself as mentor. http://trac.ffmpeg.org/wiki/SponsoringPrograms/GSoC/2015-Qualis ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/7] [GSoC] lavf: add directory listing API
On 20.03.2015 14:55, Michael Niedermayer wrote: On Fri, Mar 20, 2015 at 03:01:56AM +0100, Mariusz Szczepańczyk wrote: From: Lukasz Marek /** + * Open directory for reading. + * + * @param s directory read context. Pointer to a NULL pointer must be passed. + * @param url directory to be listed. + * @param options protocol options. + * @return >=0 on success or negative on error. + */ +int avio_open_dir(void **s, const char *url, AVDictionary **options); why void ** ? and not a more specific type ? a more specific type would allow the compiler to check types I can't remember exactly why, but as far as I remember it could be URLContext, but it is not a public struct. Also, creating dummy struct, to have it stored as void * inside is not much better, but maybe it is. (the same as AVIOContext stores it in void *opaque) ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] sftp: support reading config from ~/.ssh/config
On 11.03.2015 11:15, Nicolas George wrote: Le primidi 21 ventôse, an CCXXIII, Florian Jacob a écrit : Do we have a way to apply ffmpeg-specific options? We can force to use no compression in the code, by adding this line: ssh_options_set(libssh->session, SSH_OPTIONS_COMPRESSION, "no"); after parsing the config file. I think that is not what Reimar had in mind, but rather something like this: ssh_set_app_tag(libssh->session, "ffmpeg"); ssh_set_app_tag(libssh->session, "multimedia"); Match tag=multimedia Compression off Unfortunately, there is no "tag" criterion for the Match directive. Maybe the OpenSSH guys would be amenable to a patch. This would also mean that the user can't configure to use compression for ffmpeg even if they want to, though, but this wasn't possible before either. One might could argue that connections that benefit and work faster with the compression option are too slow to stream media, anyway. Other thoughts? I firmly believe in the following principles: 1. If the user made an explicit choice, the application must obey, even if the choice seems stupid. In the case of SSH: compression is disabled by default, so enabling it is an explicit choice. 2. Applications should as much as possible try to both leave the choice to the user and have a sensible default, but if for some reason it is not possible (poor API on a library), then priority should be given to user choice. Isn't this compression transparent? I can play media files with both compression on and off. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] sftp: support reading config from ~/.ssh/config
On 11.03.2015 01:51, Timothy Gu wrote: On Tue, Mar 10, 2015 at 5:47 PM Lukasz Marek wrote: On 11.03.2015 01:13, Florian Jacob wrote: libssh provides a function for parsing ~/.ssh/config for ssh connection parameters like user, hostname, identity file and port. This patch makes ffmpeg use this function to take parameters from the config file for everything that's not explicitely set in the url. It also supports host aliases, i.e. using a shorthand in the url and replacing it with the hostname / IP address specified for the shorthand in the config file. Works nice, pushed, thanks. Does this patch change the default behavior in any way? I thought about it, but I don't think so. If changelog consider quite minor changes then it can be mentioned tho. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] sftp: support reading config from ~/.ssh/config
On 11.03.2015 01:13, Florian Jacob wrote: libssh provides a function for parsing ~/.ssh/config for ssh connection parameters like user, hostname, identity file and port. This patch makes ffmpeg use this function to take parameters from the config file for everything that's not explicitely set in the url. It also supports host aliases, i.e. using a shorthand in the url and replacing it with the hostname / IP address specified for the shorthand in the config file. Works nice, pushed, thanks. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] sftp: support reading config from ~/.ssh/config
On 11.03.2015 00:22, Florian Jacob wrote: libssh provides a function for parsing ~/.ssh/config for ssh connection parameters like user, hostname, identity file and port. This patch makes ffmpeg use this function to take parameters from the config file for everything that's not explicitely set in the url. It also supports host aliases, i.e. using a shorthand in the url and replacing it with the hostname / IP address specified for the shorthand in the config file. Signed-off-by: Florian Jacob --- libavformat/libssh.c | 11 --- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/libavformat/libssh.c b/libavformat/libssh.c index 3ec60cb..1ed1d72 100644 --- a/libavformat/libssh.c +++ b/libavformat/libssh.c @@ -55,6 +55,10 @@ static av_cold int libssh_create_ssh_session(LIBSSHContext *libssh, const char* ssh_options_set(libssh->session, SSH_OPTIONS_TIMEOUT_USEC, &timeout); } +if (ssh_options_parse_config(libssh->session, NULL) < 0) { +av_log(libssh, AV_LOG_ERROR, "Could not parse the config file.\n"); This should be a warning (not an error), or return with error code from here. A warning is probably better. +} + if (ssh_connect(libssh->session) != SSH_OK) { av_log(libssh, AV_LOG_ERROR, "Connection failed: %s\n", ssh_get_error(libssh->session)); return AVERROR(EIO); @@ -187,7 +191,7 @@ static av_cold int libssh_open(URLContext *h, const char *url, int flags) { LIBSSHContext *libssh = h->priv_data; char proto[10], path[MAX_URL_SIZE], hostname[1024], credencials[1024]; -int port = 22, ret; +int port, ret; const char *user = NULL, *pass = NULL; char *end = NULL; @@ -198,8 +202,9 @@ static av_cold int libssh_open(URLContext *h, const char *url, int flags) path, sizeof(path), url); -if (port <= 0 || port > 65535) -port = 22; +// a port of 0 will use a port from ~/.ssh/config or the default value 22 +if (port < 0 || port > 65535) +port = 0; Is this really required? Port will be overwritten if present in config either way? if ((ret = libssh_create_ssh_session(libssh, hostname, port)) < 0) goto fail; I will test later, thanks. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avcodec/options_table: remove extradata_size from the AVOptions table
On 9 March 2015 at 14:28, Michael Niedermayer wrote: > On Mon, Mar 09, 2015 at 01:27:58PM +0100, Lukasz Marek wrote: > > On 9 March 2015 at 12:19, Andreas Cadhalpun < > > andreas.cadhal...@googlemail.com> wrote: > > > > > On 09.03.2015 03:42, Michael Niedermayer wrote: > > > > > >> allowing access to the size but not the extradata itself is not useful > > >> and could lead to potential problems if writing happens through this > field > > >> > > >> Signed-off-by: Michael Niedermayer > > >> --- > > >> libavcodec/options_table.h |1 - > > >> 1 file changed, 1 deletion(-) > > >> > > >> diff --git a/libavcodec/options_table.h b/libavcodec/options_table.h > > >> index 442b212..a906864 100644 > > >> --- a/libavcodec/options_table.h > > >> +++ b/libavcodec/options_table.h > > >> @@ -103,7 +103,6 @@ static const AVOption avcodec_options[] = { > > >> {"hex", "hex motion estimation", 0, AV_OPT_TYPE_CONST, {.i64 = > ME_HEX > > >> }, INT_MIN, INT_MAX, V|E, "me_method" }, > > >> {"umh", "umh motion estimation", 0, AV_OPT_TYPE_CONST, {.i64 = > ME_UMH > > >> }, INT_MIN, INT_MAX, V|E, "me_method" }, > > >> {"iter", "iter motion estimation", 0, AV_OPT_TYPE_CONST, {.i64 = > > >> ME_ITER }, INT_MIN, INT_MAX, V|E, "me_method" }, > > >> -{"extradata_size", NULL, OFFSET(extradata_size), AV_OPT_TYPE_INT, > {.i64 > > >> = DEFAULT }, INT_MIN, INT_MAX}, > > >> {"time_base", NULL, OFFSET(time_base), AV_OPT_TYPE_RATIONAL, {.dbl = > > >> 0}, INT_MIN, INT_MAX}, > > >> {"g", "set the group of picture (GOP) size", OFFSET(gop_size), > > >> AV_OPT_TYPE_INT, {.i64 = 12 }, INT_MIN, INT_MAX, V|E}, > > >> {"ar", "set audio sampling rate (in Hz)", OFFSET(sample_rate), > > >> AV_OPT_TYPE_INT, {.i64 = DEFAULT }, 0, INT_MAX, A|D|E}, > > >> > > >> > > > This looks very good, because it also fixes the ffm muxer to never > create > > > files with extradata_size setting, but without extradata. > > > > > > > Isn't it API break? Maybe it is better to have a look on asv1 or ffm > muxer > > to fix problem itself, not to cover it. > > I can have a look on this today's evening. > > you miss the problem, this is a demuxer side problem, > a attacker can at least crash your application no muxer side change > can fix this, the attacker has his own self written muxer that > produces a mallicious bitstream > > of course you are correct that there is possibly also a problem on the > muxer side and that this too should be fixed but that is less > important. > OK. I checked this issue and your patch fixes the issue so if you are OK with removing this option I have no more remarks. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avcodec/options_table: remove extradata_size from the AVOptions table
On 9 March 2015 at 12:19, Andreas Cadhalpun < andreas.cadhal...@googlemail.com> wrote: > On 09.03.2015 03:42, Michael Niedermayer wrote: > >> allowing access to the size but not the extradata itself is not useful >> and could lead to potential problems if writing happens through this field >> >> Signed-off-by: Michael Niedermayer >> --- >> libavcodec/options_table.h |1 - >> 1 file changed, 1 deletion(-) >> >> diff --git a/libavcodec/options_table.h b/libavcodec/options_table.h >> index 442b212..a906864 100644 >> --- a/libavcodec/options_table.h >> +++ b/libavcodec/options_table.h >> @@ -103,7 +103,6 @@ static const AVOption avcodec_options[] = { >> {"hex", "hex motion estimation", 0, AV_OPT_TYPE_CONST, {.i64 = ME_HEX >> }, INT_MIN, INT_MAX, V|E, "me_method" }, >> {"umh", "umh motion estimation", 0, AV_OPT_TYPE_CONST, {.i64 = ME_UMH >> }, INT_MIN, INT_MAX, V|E, "me_method" }, >> {"iter", "iter motion estimation", 0, AV_OPT_TYPE_CONST, {.i64 = >> ME_ITER }, INT_MIN, INT_MAX, V|E, "me_method" }, >> -{"extradata_size", NULL, OFFSET(extradata_size), AV_OPT_TYPE_INT, {.i64 >> = DEFAULT }, INT_MIN, INT_MAX}, >> {"time_base", NULL, OFFSET(time_base), AV_OPT_TYPE_RATIONAL, {.dbl = >> 0}, INT_MIN, INT_MAX}, >> {"g", "set the group of picture (GOP) size", OFFSET(gop_size), >> AV_OPT_TYPE_INT, {.i64 = 12 }, INT_MIN, INT_MAX, V|E}, >> {"ar", "set audio sampling rate (in Hz)", OFFSET(sample_rate), >> AV_OPT_TYPE_INT, {.i64 = DEFAULT }, 0, INT_MAX, A|D|E}, >> >> > This looks very good, because it also fixes the ffm muxer to never create > files with extradata_size setting, but without extradata. > Isn't it API break? Maybe it is better to have a look on asv1 or ffm muxer to fix problem itself, not to cover it. I can have a look on this today's evening. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/8] ffmdec: initialize f_cprv, f_stvi and f_stau
On 9 March 2015 at 12:41, Andreas Cadhalpun < andreas.cadhal...@googlemail.com> wrote: > On 09.03.2015 10:53, Lukasz Marek wrote: > >> In fact this is a bit wrong. COMM is guaranteed unless malformed file is >> parsed. These variables are dedicated to detect doubled sections. This >> patch allows them to occur twice in that case. So they should be >> initialized to 0. >> > > This patch doesn't change anything for valid files, it only prevents > crashes with malformed files. > > For valid files, these variables are initialized to -1, then set to 0 in > the COMM part of the switch. > > For invalid files, if another section comes before COMM, the counter is > -1, thus e.g. 'if (f_stvi++)' is true and AVERROR(EINVAL) is returned. > > If they were initialized to 0, the check wouldn't trigger for malformed > files, leading to crashes, because codec is not set. > OK. BTW, did you produced this malformed file using ffmpeg tools or just prevent theoretical case? ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/8] ffmdec: initialize f_cprv, f_stvi and f_stau
On 09.03.2015 02:45, Michael Niedermayer wrote: On Mon, Mar 09, 2015 at 12:02:55AM +0100, Andreas Cadhalpun wrote: Hi, attached patch fixes 'Conditional jump or move depends on uninitialized variables' valgrind warnings. Best regards, Andreas ffmdec.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 4a297cdd7cdb822a449cd846139a86ae284893aa 0001-ffmdec-initialize-f_cprv-f_stvi-and-f_stau.patch From 8b1088fa1509b1613d095fbe1c11eec6d251c95c Mon Sep 17 00:00:00 2001 From: Andreas Cadhalpun Date: Sun, 8 Mar 2015 22:52:47 +0100 Subject: [PATCH 1/8] ffmdec: initialize f_cprv, f_stvi and f_stau They are used in a switch statement, but it is not guaranteed that the COMM case (where they are set to 0) is reached before the other cases. applied thanks In fact this is a bit wrong. COMM is guaranteed unless malformed file is parsed. These variables are dedicated to detect doubled sections. This patch allows them to occur twice in that case. So they should be initialized to 0. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avformat/http: support auto reconnect
On 6 March 2015 at 11:19, Zhang Rui wrote: > if (!s->hd) > return AVERROR_EOF; > @@ -945,7 +949,19 @@ static int http_read_stream(URLContext *h, uint8_t > *buf, int size) > if (s->compressed) > return http_buf_read_compressed(h, buf, size); > #endif /* CONFIG_ZLIB */ > -return http_buf_read(h, buf, size); > +read_ret = http_buf_read(h, buf, size); > +if (s->reconnect && s->filesize > 0 && s->off < s->filesize && > read_ret < 0) { > minor: you can chek read_ret < 0 first, this condition usually will net be meet. > +av_log(h, AV_LOG_WARNING, "Will reconnect at %"PRId64".\n", > s->off); > Not sure this should be a warning. maybe info, debug or verbose? > +seek_ret = http_seek_internal(h, s->off, SEEK_SET, 1); > +if (seek_ret != s->off) { > +av_log(h, AV_LOG_WARNING, "Failed to reconnect at > %"PRId64".\n", s->off); > This should be an error I think. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [RFC][PATCH] ffplay: factorize subtitle rendering code to a private filter
On 3 March 2015 at 14:16, wm4 wrote: > On Tue, 3 Mar 2015 13:17:12 +0100 > Lukasz Marek wrote: > > > On 17 February 2015 at 03:27, Marton Balint wrote: > > > > > > > > > > > On Sat, 14 Feb 2015, Clément Bœsch wrote: > > > > > > On Sat, Feb 14, 2015 at 02:08:15AM +0100, Marton Balint wrote: > > >> > > >>> Signed-off-by: Marton Balint > > >>> --- > > >>> Makefile | 1 + > > >>> doc/ffplay.texi | 4 + > > >>> ffplay.c | 336 > +- > > >>> --- > > >>> libavfilter/Makefile | 1 + > > >>> libavfilter/vf_ffplay_subtitle.c | 347 > ++ > > >>> + > > >>> libavfilter/vf_ffplay_subtitle.h | 38 + > > >>> 6 files changed, 434 insertions(+), 293 deletions(-) > > >>> create mode 100644 libavfilter/vf_ffplay_subtitle.c > > >>> create mode 100644 libavfilter/vf_ffplay_subtitle.h > > >>> > > >>> > > >> This is not the correct way of doing it. > > >> > > >> The proper way is to integrate subtitles within lavfi. If you want to > work > > >> toward this, feel free to ask for more details, but it's not trivial. > > >> > > >> (and no, this patch is not an improvement in the correct direction, > sorry) > > >> > > >> > > > What would be an improvement in the right direction? I see little > chance > > > that I will work on subtitles in lavfi, but if there is a > better/preferred > > > way to change ffplay to support subtitles in the filter chain without > > > waiting for the whole subtitles-in-libavfilter infrastructure, I may be > > > interested in working toward that. > > > > > > > Just random guess, but for now you could implement this filter in > > ffplay_sub_filter.c (or something) and register it from ffplay. So code > of > > ffplay itself is cleaner and libavfilter is not spoiled. > > When this all suff Clement is talking about is done then it can be pushed > > further. > > > > I like this patch because some time ago I ported ffplay to ios and > removing > > this code was annoying. > > API users can't add new filters from the "outside". Isn't it allowed (and enough) to call avfilter_register with own filter? If not then I,m surprised TBH. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [RFC][PATCH] ffplay: factorize subtitle rendering code to a private filter
On 17 February 2015 at 03:27, Marton Balint wrote: > > > On Sat, 14 Feb 2015, Clément Bœsch wrote: > > On Sat, Feb 14, 2015 at 02:08:15AM +0100, Marton Balint wrote: >> >>> Signed-off-by: Marton Balint >>> --- >>> Makefile | 1 + >>> doc/ffplay.texi | 4 + >>> ffplay.c | 336 +- >>> --- >>> libavfilter/Makefile | 1 + >>> libavfilter/vf_ffplay_subtitle.c | 347 ++ >>> + >>> libavfilter/vf_ffplay_subtitle.h | 38 + >>> 6 files changed, 434 insertions(+), 293 deletions(-) >>> create mode 100644 libavfilter/vf_ffplay_subtitle.c >>> create mode 100644 libavfilter/vf_ffplay_subtitle.h >>> >>> >> This is not the correct way of doing it. >> >> The proper way is to integrate subtitles within lavfi. If you want to work >> toward this, feel free to ask for more details, but it's not trivial. >> >> (and no, this patch is not an improvement in the correct direction, sorry) >> >> > What would be an improvement in the right direction? I see little chance > that I will work on subtitles in lavfi, but if there is a better/preferred > way to change ffplay to support subtitles in the filter chain without > waiting for the whole subtitles-in-libavfilter infrastructure, I may be > interested in working toward that. > Just random guess, but for now you could implement this filter in ffplay_sub_filter.c (or something) and register it from ffplay. So code of ffplay itself is cleaner and libavfilter is not spoiled. When this all suff Clement is talking about is done then it can be pushed further. I like this patch because some time ago I ported ffplay to ios and removing this code was annoying. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] lavf/ffmenc: do not fail on missing codec
On 20.02.2015 01:53, Michael Niedermayer wrote: On Thu, Feb 19, 2015 at 11:52:50PM +0100, Lukasz Marek wrote: ffm encoder fails when codec is not found. It may happen when stream is being copied. This commit allows to store such stream and provides backward compatibility with version prior 2.5 release. fixes #4266 Signed-off-by: Lukasz Marek LGTM pushed ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 2/2] hlsenc: write playlist into a temp file and replace the original atomically
On 20.02.2015 14:30, Michael Niedermayer wrote: On Fri, Feb 20, 2015 at 02:15:30PM +0100, Hendrik Leppkes wrote: On Fri, Feb 20, 2015 at 1:38 PM, Michael Niedermayer wrote: On Fri, Feb 20, 2015 at 12:55:14PM +0100, Hendrik Leppkes wrote: What else could it be? http, ftp, who knows also ff_rename() would interpret a "http://"; differently from avio_open2(), so something more is needed to not end up with odd rename() calls It writes to separate segment files as well, not sure that would work on anything but local files. i dont know either but if someone passes "ftp://..."; it should either work or fail with a error message or mostly work while printing a warning Some ftp servers claim thay allow to upload atomically. Who knows whats the true. I wouldn't do that probably. But real regression is in case someone doesn't really care to have live stream, but just want to generate manifest and chunks on remote server directly for later use. For me you patch is OK, but maybe it is worth to add rename as part of protocol implementation to support these cases too. Assuming tempfile could have hardcoded name like ".filename" instead of "filename" and rename it when is ready. For the record, this is the same method used in dashenc for writing its manifest file, and it can avoid a sort-of race condition when serving the files through a web server directly (ie. web server reading while in the middle of re-writing it). And the same in smoothstreaming encoder. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] lavf/ffmenc: do not fail on missing codec
ffm encoder fails when codec is not found. It may happen when stream is being copied. This commit allows to store such stream and provides backward compatibility with version prior 2.5 release. fixes #4266 Signed-off-by: Lukasz Marek --- libavformat/ffmenc.c | 13 - 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/libavformat/ffmenc.c b/libavformat/ffmenc.c index 10acfbe..adb5019 100644 --- a/libavformat/ffmenc.c +++ b/libavformat/ffmenc.c @@ -94,15 +94,18 @@ static void write_header_chunk(AVIOContext *pb, AVIOContext *dpb, unsigned id) av_free(dyn_buf); } -static int ffm_write_header_codec_private_ctx(AVIOContext *pb, AVCodecContext *ctx, int type) +static int ffm_write_header_codec_private_ctx(AVFormatContext *s, AVCodecContext *ctx, int type) { +AVIOContext *pb = s->pb; AVIOContext *tmp; char *buf = NULL; int ret; const AVCodec *enc = ctx->codec ? ctx->codec : avcodec_find_encoder(ctx->codec_id); -if (!enc) -return AVERROR(EINVAL); +if (!enc) { +av_log(s, AV_LOG_WARNING, "Stream codec is not found. Codec private options are not stored.\n"); +return 0; +} if (ctx->priv_data && enc->priv_class && enc->priv_data_size) { if ((ret = av_opt_serialize(ctx->priv_data, AV_OPT_FLAG_ENCODING_PARAM | type, AV_OPT_SERIALIZE_SKIP_DEFAULTS, &buf, '=', ',')) < 0) @@ -281,7 +284,7 @@ static int ffm_write_header(AVFormatContext *s) st->recommended_encoder_configuration)) < 0) return ret; } else if ((ret = ffm_write_header_codec_ctx(s->pb, codec, MKBETAG('S', '2', 'V', 'I'), AV_OPT_FLAG_VIDEO_PARAM)) < 0 || - (ret = ffm_write_header_codec_private_ctx(s->pb, codec, AV_OPT_FLAG_VIDEO_PARAM)) < 0) + (ret = ffm_write_header_codec_private_ctx(s, codec, AV_OPT_FLAG_VIDEO_PARAM)) < 0) return ret; break; case AVMEDIA_TYPE_AUDIO: @@ -292,7 +295,7 @@ static int ffm_write_header(AVFormatContext *s) st->recommended_encoder_configuration)) < 0) return ret; } else if ((ret = ffm_write_header_codec_ctx(s->pb, codec, MKBETAG('S', '2', 'A', 'U'), AV_OPT_FLAG_AUDIO_PARAM)) < 0 || - (ret = ffm_write_header_codec_private_ctx(s->pb, codec, AV_OPT_FLAG_AUDIO_PARAM)) < 0) + (ret = ffm_write_header_codec_private_ctx(s, codec, AV_OPT_FLAG_AUDIO_PARAM)) < 0) return ret; break; default: -- 1.9.1 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] lavf/avc: add buffer padding to extradata allocation
On 05.02.2015 03:04, Michael Niedermayer wrote: On Thu, Feb 05, 2015 at 01:06:33AM +0100, Lukasz Marek wrote: ff_avc_write_annexb_extradata() allocates extradata, but don't add FF_INPUT_BUFFER_PADDING_SIZE value Signed-off-by: Lukasz Marek --- libavformat/avc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) LGTM pushed ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] lavf/avc: add buffer padding to extradata allocation
ff_avc_write_annexb_extradata() allocates extradata, but don't add FF_INPUT_BUFFER_PADDING_SIZE value Signed-off-by: Lukasz Marek --- libavformat/avc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libavformat/avc.c b/libavformat/avc.c index c927b47..9d843e0 100644 --- a/libavformat/avc.c +++ b/libavformat/avc.c @@ -180,7 +180,7 @@ int ff_avc_write_annexb_extradata(const uint8_t *in, uint8_t **buf, int *size) if (11 + sps_size + pps_size > *size) return AVERROR_INVALIDDATA; out_size = 8 + sps_size + pps_size; -out = av_mallocz(out_size); +out = av_mallocz(out_size + FF_INPUT_BUFFER_PADDING_SIZE); if (!out) return AVERROR(ENOMEM); AV_WB32(&out[0], 0x0001); -- 1.9.1 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] Revert "Autodetect libxcb."
On 04.02.2015 21:39, Carl Eugen Hoyos wrote: Clément Bœsch pkh.me> writes: This reverts commit 23ec8db8a07467a1fbef0c79f16b33040ca63c24. There is no reason to enable libxcb by default. This is inconsistent with all the other libs (we don't do it for x11, opengl, ...) x11grab is not lgpl (that is what you meant with x11, no?) and opengl is a slightly obscure feature (but please enable auto-detection if you think that opengl is a system library). I have not opinion what's better: to explicitly enable or disable a feature. Autodetecting has one disadvantage though, it has to work perfectly on every possible platform. Otherwise library may be detected, but may be not sufficient for some reason. So it is harder to maintain. Project that detect library and don't compile smells bad. And opengl is good example of the library that may be not easy to autodetect. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] doc/indevs: add xcbgrab
On 27 January 2015 at 22:48, Lou Logan wrote: > Signed-off-by: Lou Logan > --- > doc/indevs.texi | 10 ++ > 1 file changed, 10 insertions(+) > > diff --git a/doc/indevs.texi b/doc/indevs.texi > index ae61331..350adfe 100644 > --- a/doc/indevs.texi > +++ b/doc/indevs.texi > @@ -971,6 +971,7 @@ The filename passed as input is the capture driver > number, ranging from > 0 to 9. You may use "list" as filename to print a list of drivers. Any > other filename will be interpreted as device number 0. > > +@anchor{x11grab} > @section x11grab > > X11 video input device. > @@ -1064,6 +1065,15 @@ Use the MIT-SHM extension for shared memory. > Default value is @code{1}. > It may be necessary to disable it for remote displays. > @end table > > +@section xcbgrab > + > +XCB based X11 video input device. > + > +To enable this input device during configuration you need > +libxcb installed on your system. > + > +Options are identical to @ref{x11grab}. > > I'm not sure this is intended, but x11grab and xcbgrab are exclusive and cannot be present aside: git grep '"x11grab"' x11grab.c:.name = "x11grab", xcbgrab.c:.name = "x11grab", Maybe it is worth to mention it somewhere to not confuse users. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [RFC] Support dynamic loading of third-party libs
On 13 January 2015 at 14:54, wm4 wrote: > On Mon, 12 Jan 2015 18:59:33 +0100 > Marc Giger wrote: > > > Hi, > > > > Attached is a preliminary patch that enables runtime loading of external > > libraries via dlopen and friends. Dynamic loading is a build time option > > (--enable-dynamic-loading) and if it is not activated the libs are > > linked as usual (still no dependency to dlopen required). > > > > The patch is intended as a basis for a discussion and therefore the > > following applies: > > > > - only libmp3lame and libva will by dynamically loaded atm > > - only tested on linux atm > > - deregistering not implemented (dlclose) > > - versioning (which version of a lib should be loaded?) > > - library usage counter missing > > - ...? > > > > What do you think? > > What's the point? > Loading library "on demand" saves some time on application start and possibly saves memory. I is not relevant, on modern desktops, but on mobile devices it can make a difference. I haven't looked at patch, but idea as a whole is OK IMHO ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/2] lavd/v4l2: implement list device callback
On 03.01.2015 14:51, Giorgio Vazzana wrote: 2015-01-03 4:45 GMT+01:00 Lukasz Marek : On 21 December 2014 at 23:39, Lukasz Marek wrote: On 21.12.2014 22:43, Lukasz Marek wrote: Signed-off-by: Lukasz Marek --- libavdevice/v4l2.c | 58 ++ 1 file changed, 58 insertions(+) diff --git a/libavdevice/v4l2.c b/libavdevice/v4l2.c index 2969980..9d4d7ae 100644 --- a/libavdevice/v4l2.c +++ b/libavdevice/v4l2.c @@ -1006,6 +1006,63 @@ static int v4l2_read_close(AVFormatContext *ctx) return 0; } +static int v4l2_get_device_list(AVFormatContext *ctx, AVDeviceInfoList *device_list) +{ +struct video_data *s = ctx->priv_data; +AVDeviceInfo *device = NULL; +struct v4l2_capability cap; +int i, ret = 0; + +if (!device_list) +return AVERROR(EINVAL); + +for (i = 0; i <= 31; i++) { +snprintf(ctx->filename, sizeof(ctx->filename), "/dev/video%d", i); I wasn't sure this is correct. I changed this loop to opendir/readdir - similar way v4l-utils does. ping on patchset LGTM, thanks. pushed both patches ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 2/2] cmdutils: use helper functions for listing sinks/sources
On 03.01.2015 06:35, Michael Niedermayer wrote: On Sat, Jan 03, 2015 at 04:45:56AM +0100, Lukasz Marek wrote: On 21 December 2014 at 21:04, Lukasz Marek wrote: List device callback must be able to return valid list without opening device. This callback should return input values for open function, not vice-versa. Read header funtion is very likey to fail without proper configuration provided. Signed-off-by: Lukasz Marek --- cmdutils.c | 27 ++- 1 file changed, 2 insertions(+), 25 deletions(-) ping on patchset in absence of other comments, i think these 2 patches should be ok pushed both ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] lavd/avdevice: remove av_ prefix from private function
On 22.12.2014 01:14, Michael Niedermayer wrote: On Sun, Dec 21, 2014 at 09:03:49PM +0100, Lukasz Marek wrote: Signed-off-by: Lukasz Marek --- libavdevice/avdevice.c | 20 ++-- 1 file changed, 10 insertions(+), 10 deletions(-) LGTM pushed ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 2/2] cmdutils: use helper functions for listing sinks/sources
On 21 December 2014 at 21:04, Lukasz Marek wrote: > List device callback must be able to return valid list without opening > device. > This callback should return input values for open function, not vice-versa. > Read header funtion is very likey to fail without proper configuration > provided. > > Signed-off-by: Lukasz Marek > --- > cmdutils.c | 27 ++- > 1 file changed, 2 insertions(+), 25 deletions(-) > ping on patchset ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/2] lavd/v4l2: implement list device callback
On 21 December 2014 at 23:39, Lukasz Marek wrote: > On 21.12.2014 22:43, Lukasz Marek wrote: > >> Signed-off-by: Lukasz Marek >> --- >> libavdevice/v4l2.c | 58 ++ >> >> 1 file changed, 58 insertions(+) >> >> diff --git a/libavdevice/v4l2.c b/libavdevice/v4l2.c >> index 2969980..9d4d7ae 100644 >> --- a/libavdevice/v4l2.c >> +++ b/libavdevice/v4l2.c >> @@ -1006,6 +1006,63 @@ static int v4l2_read_close(AVFormatContext *ctx) >> return 0; >> } >> >> +static int v4l2_get_device_list(AVFormatContext *ctx, AVDeviceInfoList >> *device_list) >> +{ >> +struct video_data *s = ctx->priv_data; >> +AVDeviceInfo *device = NULL; >> +struct v4l2_capability cap; >> +int i, ret = 0; >> + >> +if (!device_list) >> +return AVERROR(EINVAL); >> + >> +for (i = 0; i <= 31; i++) { >> +snprintf(ctx->filename, sizeof(ctx->filename), "/dev/video%d", >> i); >> > > I wasn't sure this is correct. I changed this loop to opendir/readdir - > similar way v4l-utils does. > > ping on patchset ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/2] lavd/v4l2: implement list device callback
On 21.12.2014 22:43, Lukasz Marek wrote: Signed-off-by: Lukasz Marek --- libavdevice/v4l2.c | 58 ++ 1 file changed, 58 insertions(+) diff --git a/libavdevice/v4l2.c b/libavdevice/v4l2.c index 2969980..9d4d7ae 100644 --- a/libavdevice/v4l2.c +++ b/libavdevice/v4l2.c @@ -1006,6 +1006,63 @@ static int v4l2_read_close(AVFormatContext *ctx) return 0; } +static int v4l2_get_device_list(AVFormatContext *ctx, AVDeviceInfoList *device_list) +{ +struct video_data *s = ctx->priv_data; +AVDeviceInfo *device = NULL; +struct v4l2_capability cap; +int i, ret = 0; + +if (!device_list) +return AVERROR(EINVAL); + +for (i = 0; i <= 31; i++) { +snprintf(ctx->filename, sizeof(ctx->filename), "/dev/video%d", i); I wasn't sure this is correct. I changed this loop to opendir/readdir - similar way v4l-utils does. >From e2efb8e8b803b461d5c27b22435318e38639a2c8 Mon Sep 17 00:00:00 2001 From: Lukasz Marek Date: Sun, 21 Dec 2014 22:37:18 +0100 Subject: [PATCH] lavd/v4l2: implement list device callback Signed-off-by: Lukasz Marek --- libavdevice/v4l2.c | 77 ++ 1 file changed, 77 insertions(+) diff --git a/libavdevice/v4l2.c b/libavdevice/v4l2.c index 2969980..8337cf5 100644 --- a/libavdevice/v4l2.c +++ b/libavdevice/v4l2.c @@ -31,6 +31,7 @@ */ #include "v4l2-common.h" +#include #if CONFIG_LIBV4L2 #include @@ -1006,6 +1007,81 @@ static int v4l2_read_close(AVFormatContext *ctx) return 0; } +static int v4l2_is_v4l_dev(const char *name) +{ +return !strncmp(name, "video", 5) || + !strncmp(name, "radio", 5) || + !strncmp(name, "vbi", 3) || + !strncmp(name, "v4l-subdev", 10); +} + +static int v4l2_get_device_list(AVFormatContext *ctx, AVDeviceInfoList *device_list) +{ +struct video_data *s = ctx->priv_data; +DIR *dir; +struct dirent *entry; +AVDeviceInfo *device = NULL; +struct v4l2_capability cap; +int ret = 0; + +if (!device_list) +return AVERROR(EINVAL); + +dir = opendir("/dev"); +if (!dir) { +ret = AVERROR(errno); +av_log(ctx, AV_LOG_ERROR, "Couldn't open the directory: %s\n", av_err2str(ret)); +return ret; +} +while ((entry = readdir(dir))) { +if (!v4l2_is_v4l_dev(entry->d_name)) +continue; + +snprintf(ctx->filename, sizeof(ctx->filename), "/dev/%s", entry->d_name); +if ((s->fd = device_open(ctx)) < 0) +continue; + +if (v4l2_ioctl(s->fd, VIDIOC_QUERYCAP, &cap) < 0) { +ret = AVERROR(errno); +av_log(ctx, AV_LOG_ERROR, "ioctl(VIDIOC_QUERYCAP): %s\n", av_err2str(ret)); +goto fail; +} + +device = av_mallocz(sizeof(AVDeviceInfo)); +if (!device) { +ret = AVERROR(ENOMEM); +goto fail; +} +device->device_name = av_strdup(ctx->filename); +device->device_description = av_strdup(cap.card); +if (!device->device_name || !device->device_description) { +ret = AVERROR(ENOMEM); +goto fail; +} + +if ((ret = av_dynarray_add_nofree(&device_list->devices, + &device_list->nb_devices, device)) < 0) +goto fail; + +v4l2_close(s->fd); +s->fd = -1; +continue; + + fail: +if (device) { +av_freep(&device->device_name); +av_freep(&device->device_description); +av_freep(&device); +} +if (s->fd >= 0) +v4l2_close(s->fd); +s->fd = -1; +break; +} +closedir(dir); +return ret; +} + #define OFFSET(x) offsetof(struct video_data, x) #define DEC AV_OPT_FLAG_DECODING_PARAM @@ -1050,6 +1126,7 @@ AVInputFormat ff_v4l2_demuxer = { .read_header= v4l2_read_header, .read_packet= v4l2_read_packet, .read_close = v4l2_read_close, +.get_device_list = v4l2_get_device_list, .flags = AVFMT_NOFILE, .priv_class = &v4l2_class, }; -- 1.9.1 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 2/2] cmdutils: use av_match_name to filter devices
Device name may be coma-separated list. Use dedicated funtion to compare. Signed-off-by: Lukasz Marek --- cmdutils.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/cmdutils.c b/cmdutils.c index fb1883a..4a4f311 100644 --- a/cmdutils.c +++ b/cmdutils.c @@ -2173,7 +2173,7 @@ int show_sources(void *optctx, const char *opt, const char *arg) if (fmt) { if (!strcmp(fmt->name, "lavfi")) continue; //it's pointless to probe lavfi -if (dev && strcmp(fmt->name, dev)) +if (dev && !av_match_name(dev, fmt->name)) continue; print_device_sources(fmt, opts); } @@ -2181,7 +2181,7 @@ int show_sources(void *optctx, const char *opt, const char *arg) do { fmt = av_input_video_device_next(fmt); if (fmt) { -if (dev && strcmp(fmt->name, dev)) +if (dev && !av_match_name(dev, fmt->name)) continue; print_device_sources(fmt, opts); } @@ -2209,7 +2209,7 @@ int show_sinks(void *optctx, const char *opt, const char *arg) do { fmt = av_output_audio_device_next(fmt); if (fmt) { -if (dev && strcmp(fmt->name, dev)) +if (dev && !av_match_name(dev, fmt->name)) continue; print_device_sinks(fmt, opts); } @@ -2217,7 +2217,7 @@ int show_sinks(void *optctx, const char *opt, const char *arg) do { fmt = av_output_video_device_next(fmt); if (fmt) { -if (dev && strcmp(fmt->name, dev)) +if (dev && !av_match_name(dev, fmt->name)) continue; print_device_sinks(fmt, opts); } -- 1.9.1 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 1/2] lavd/v4l2: implement list device callback
Signed-off-by: Lukasz Marek --- libavdevice/v4l2.c | 58 ++ 1 file changed, 58 insertions(+) diff --git a/libavdevice/v4l2.c b/libavdevice/v4l2.c index 2969980..9d4d7ae 100644 --- a/libavdevice/v4l2.c +++ b/libavdevice/v4l2.c @@ -1006,6 +1006,63 @@ static int v4l2_read_close(AVFormatContext *ctx) return 0; } +static int v4l2_get_device_list(AVFormatContext *ctx, AVDeviceInfoList *device_list) +{ +struct video_data *s = ctx->priv_data; +AVDeviceInfo *device = NULL; +struct v4l2_capability cap; +int i, ret = 0; + +if (!device_list) +return AVERROR(EINVAL); + +for (i = 0; i <= 31; i++) { +snprintf(ctx->filename, sizeof(ctx->filename), "/dev/video%d", i); + +if (avio_check(ctx->filename, AVIO_FLAG_READ) != AVIO_FLAG_READ || +(s->fd = device_open(ctx)) < 0) +continue; + +if (v4l2_ioctl(s->fd, VIDIOC_QUERYCAP, &cap) < 0) { +ret = AVERROR(errno); +av_log(ctx, AV_LOG_ERROR, "ioctl(VIDIOC_QUERYCAP): %s\n", av_err2str(ret)); +goto fail_device; +} + +device = av_mallocz(sizeof(AVDeviceInfo)); +if (!device) { +ret = AVERROR(ENOMEM); +goto fail_device; +} +device->device_name = av_strdup(ctx->filename); +device->device_description = av_strdup(cap.card); +if (!device->device_name || !device->device_description) { +ret = AVERROR(ENOMEM); +goto fail_device; +} + +if ((ret = av_dynarray_add_nofree(&device_list->devices, + &device_list->nb_devices, device)) < 0) +goto fail_device; + +v4l2_close(s->fd); +s->fd = -1; +continue; + + fail_device: +if (device) { +av_freep(&device->device_name); +av_freep(&device->device_description); +av_freep(&device); +} +if (s->fd >= 0) +v4l2_close(s->fd); +s->fd = -1; +return ret; +} +return 0; +} + #define OFFSET(x) offsetof(struct video_data, x) #define DEC AV_OPT_FLAG_DECODING_PARAM @@ -1050,6 +1107,7 @@ AVInputFormat ff_v4l2_demuxer = { .read_header= v4l2_read_header, .read_packet= v4l2_read_packet, .read_close = v4l2_read_close, +.get_device_list = v4l2_get_device_list, .flags = AVFMT_NOFILE, .priv_class = &v4l2_class, }; -- 1.9.1 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 2/2] cmdutils: use helper functions for listing sinks/sources
List device callback must be able to return valid list without opening device. This callback should return input values for open function, not vice-versa. Read header funtion is very likey to fail without proper configuration provided. Signed-off-by: Lukasz Marek --- cmdutils.c | 27 ++- 1 file changed, 2 insertions(+), 25 deletions(-) diff --git a/cmdutils.c b/cmdutils.c index 2b4ab9e..fb1883a 100644 --- a/cmdutils.c +++ b/cmdutils.c @@ -2076,9 +2076,7 @@ void *grow_array(void *array, int elem_size, int *size, int new_size) static int print_device_sources(AVInputFormat *fmt, AVDictionary *opts) { int ret, i; -AVFormatContext *dev = NULL; AVDeviceInfoList *device_list = NULL; -AVDictionary *tmp_opts = NULL; if (!fmt || !fmt->priv_class || !AV_IS_INPUT_DEVICE(fmt->priv_class->category)) return AVERROR(EINVAL); @@ -2090,15 +2088,7 @@ static int print_device_sources(AVInputFormat *fmt, AVDictionary *opts) goto fail; } -/* TODO: avformat_open_input calls read_header callback which is not necessary. - Function like avformat_alloc_output_context2 for input could be helpful here. */ -av_dict_copy(&tmp_opts, opts, 0); -if ((ret = avformat_open_input(&dev, NULL, fmt, &tmp_opts)) < 0) { -printf("Cannot open device: %s.\n", fmt->name); -goto fail; -} - -if ((ret = avdevice_list_devices(dev, &device_list)) < 0) { +if ((ret = avdevice_list_input_sources(fmt, NULL, opts, &device_list)) < 0) { printf("Cannot list sources.\n"); goto fail; } @@ -2109,18 +2099,14 @@ static int print_device_sources(AVInputFormat *fmt, AVDictionary *opts) } fail: -av_dict_free(&tmp_opts); avdevice_free_list_devices(&device_list); -avformat_close_input(&dev); return ret; } static int print_device_sinks(AVOutputFormat *fmt, AVDictionary *opts) { int ret, i; -AVFormatContext *dev = NULL; AVDeviceInfoList *device_list = NULL; -AVDictionary *tmp_opts = NULL; if (!fmt || !fmt->priv_class || !AV_IS_OUTPUT_DEVICE(fmt->priv_class->category)) return AVERROR(EINVAL); @@ -2132,14 +2118,7 @@ static int print_device_sinks(AVOutputFormat *fmt, AVDictionary *opts) goto fail; } -if ((ret = avformat_alloc_output_context2(&dev, fmt, NULL, NULL)) < 0) { -printf("Cannot open device: %s.\n", fmt->name); -goto fail; -} -av_dict_copy(&tmp_opts, opts, 0); -av_opt_set_dict2(dev, &tmp_opts, AV_OPT_SEARCH_CHILDREN); - -if ((ret = avdevice_list_devices(dev, &device_list)) < 0) { +if ((ret = avdevice_list_output_sinks(fmt, NULL, opts, &device_list)) < 0) { printf("Cannot list sinks.\n"); goto fail; } @@ -2150,9 +2129,7 @@ static int print_device_sinks(AVOutputFormat *fmt, AVDictionary *opts) } fail: -av_dict_free(&tmp_opts); avdevice_free_list_devices(&device_list); -avformat_free_context(dev); return ret; } -- 1.9.1 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] lavd/avdevice: remove av_ prefix from private function
Signed-off-by: Lukasz Marek --- libavdevice/avdevice.c | 20 ++-- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/libavdevice/avdevice.c b/libavdevice/avdevice.c index d59e99a..72e573d 100644 --- a/libavdevice/avdevice.c +++ b/libavdevice/avdevice.c @@ -77,8 +77,8 @@ const char * avdevice_license(void) return LICENSE_PREFIX FFMPEG_LICENSE + sizeof(LICENSE_PREFIX) - 1; } -static void *av_device_next(void *prev, int output, -AVClassCategory c1, AVClassCategory c2) +static void *device_next(void *prev, int output, + AVClassCategory c1, AVClassCategory c2) { const AVClass *pc; AVClassCategory category = AV_CLASS_CATEGORY_NA; @@ -101,26 +101,26 @@ static void *av_device_next(void *prev, int output, AVInputFormat *av_input_audio_device_next(AVInputFormat *d) { -return av_device_next(d, 0, AV_CLASS_CATEGORY_DEVICE_AUDIO_INPUT, - AV_CLASS_CATEGORY_DEVICE_INPUT); +return device_next(d, 0, AV_CLASS_CATEGORY_DEVICE_AUDIO_INPUT, + AV_CLASS_CATEGORY_DEVICE_INPUT); } AVInputFormat *av_input_video_device_next(AVInputFormat *d) { -return av_device_next(d, 0, AV_CLASS_CATEGORY_DEVICE_VIDEO_INPUT, - AV_CLASS_CATEGORY_DEVICE_INPUT); +return device_next(d, 0, AV_CLASS_CATEGORY_DEVICE_VIDEO_INPUT, + AV_CLASS_CATEGORY_DEVICE_INPUT); } AVOutputFormat *av_output_audio_device_next(AVOutputFormat *d) { -return av_device_next(d, 1, AV_CLASS_CATEGORY_DEVICE_AUDIO_OUTPUT, - AV_CLASS_CATEGORY_DEVICE_OUTPUT); +return device_next(d, 1, AV_CLASS_CATEGORY_DEVICE_AUDIO_OUTPUT, + AV_CLASS_CATEGORY_DEVICE_OUTPUT); } AVOutputFormat *av_output_video_device_next(AVOutputFormat *d) { -return av_device_next(d, 1, AV_CLASS_CATEGORY_DEVICE_VIDEO_OUTPUT, - AV_CLASS_CATEGORY_DEVICE_OUTPUT); +return device_next(d, 1, AV_CLASS_CATEGORY_DEVICE_VIDEO_OUTPUT, + AV_CLASS_CATEGORY_DEVICE_OUTPUT); } int avdevice_app_to_dev_control_message(struct AVFormatContext *s, enum AVAppToDevMessageType type, -- 1.9.1 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 1/2] lavd/avdevice: introduce helper functions for sink/sources listing
TODO: bump minor, update doc/APIchanges Signed-off-by: Lukasz Marek --- libavdevice/Makefile | 1 + libavdevice/avdevice.c | 39 + libavdevice/avdevice.h | 22 +++ libavdevice/internal.h | 27 +++ libavdevice/utils.c| 59 ++ 5 files changed, 148 insertions(+) create mode 100644 libavdevice/internal.h create mode 100644 libavdevice/utils.c diff --git a/libavdevice/Makefile b/libavdevice/Makefile index 6b8ab2e..872504b 100644 --- a/libavdevice/Makefile +++ b/libavdevice/Makefile @@ -7,6 +7,7 @@ HEADERS = avdevice.h \ OBJS= alldevices.o \ avdevice.o\ + utils.o \ # input/output devices OBJS-$(CONFIG_ALSA_INDEV)+= alsa-audio-common.o \ diff --git a/libavdevice/avdevice.c b/libavdevice/avdevice.c index 72e573d..72e1b67 100644 --- a/libavdevice/avdevice.c +++ b/libavdevice/avdevice.c @@ -21,6 +21,7 @@ #include "libavutil/pixfmt.h" #include "libavcodec/avcodec.h" #include "avdevice.h" +#include "internal.h" #include "config.h" #include "libavutil/ffversion.h" @@ -208,6 +209,44 @@ int avdevice_list_devices(AVFormatContext *s, AVDeviceInfoList **device_list) return ret; } +static int list_devices_for_context(AVFormatContext *s, AVDictionary *options, +AVDeviceInfoList **device_list) +{ +AVDictionary *tmp = NULL; +int ret; + +av_dict_copy(&tmp, options, 0); +if ((ret = av_opt_set_dict2(s, &tmp, AV_OPT_SEARCH_CHILDREN)) < 0) +goto fail; +ret = avdevice_list_devices(s, device_list); + fail: +av_dict_free(&tmp); +avformat_free_context(s); +return ret; +} + +int avdevice_list_input_sources(AVInputFormat *device, const char *device_name, +AVDictionary *device_options, AVDeviceInfoList **device_list) +{ +AVFormatContext *s = NULL; +int ret; + +if ((ret = ff_alloc_input_device_context(&s, device, device_name)) < 0) +return ret; +return list_devices_for_context(s, device_options, device_list); +} + +int avdevice_list_output_sinks(AVOutputFormat *device, const char *device_name, + AVDictionary *device_options, AVDeviceInfoList **device_list) +{ +AVFormatContext *s = NULL; +int ret; + +if ((ret = avformat_alloc_output_context2(&s, device, device_name, NULL)) < 0) +return ret; +return list_devices_for_context(s, device_options, device_list); +} + void avdevice_free_list_devices(AVDeviceInfoList **device_list) { AVDeviceInfoList *list; diff --git a/libavdevice/avdevice.h b/libavdevice/avdevice.h index a395228..2d675b0 100644 --- a/libavdevice/avdevice.h +++ b/libavdevice/avdevice.h @@ -484,4 +484,26 @@ int avdevice_list_devices(struct AVFormatContext *s, AVDeviceInfoList **device_l */ void avdevice_free_list_devices(AVDeviceInfoList **device_list); +/** + * List devices. + * + * Returns available device names and their parameters. + * These are convinient wrappers for avdevice_list_devices(). + * Device context is allocated and deallocated internally. + * + * @param device device format. May be NULL if device name is set. + * @param device_name device name. May be NULL if device format is set. + * @param device_options An AVDictionary filled with device-private options. May be NULL. + * The same options must be passed later to avformat_write_header() for output + * devices or avformat_open_input() for input devices, or at any other place + * that affects device-private options. + * @param[out] device_list list of autodetected devices + * @return count of autodetected devices, negative on error. + * @note device argument takes precedence over device_name when both are set. + */ +int avdevice_list_input_sources(struct AVInputFormat *device, const char *device_name, +AVDictionary *device_options, AVDeviceInfoList **device_list); +int avdevice_list_output_sinks(struct AVOutputFormat *device, const char *device_name, + AVDictionary *device_options, AVDeviceInfoList **device_list); + #endif /* AVDEVICE_AVDEVICE_H */ diff --git a/libavdevice/internal.h b/libavdevice/internal.h new file mode 100644 index 000..3cd1b06 --- /dev/null +++ b/libavdevice/internal.h @@ -0,0 +1,27 @@ +/* + * This file is part of FFmpeg. + * + * FFmpeg is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; eit
Re: [FFmpeg-devel] [PATCH] cmdutils: dont call read_header before listing devices
On 18 December 2014 at 10:41, Michael Niedermayer wrote: > > On Thu, Dec 18, 2014 at 01:29:39AM +0100, Lukasz Marek wrote: > > On 18.12.2014 01:09, Michael Niedermayer wrote: > > >On Wed, Dec 17, 2014 at 10:59:37PM +0100, Lukasz Marek wrote: > > >>On 15.12.2014 14:18, Michael Niedermayer wrote: > > >>>> cmdutils.c |8 ++-- > > >>>> 1 file changed, 6 insertions(+), 2 deletions(-) > > >>>>8d012a5193b0440717f89d920661913ef160e674 > 0001-cmdutils-dont-call-read_header-before-listing-device.patch > > >>>> From 332bb7456c498518ea72dfdaa0e8c3e76d383f21 Mon Sep 17 00:00:00 > 2001 > > >>>>From: Lukasz Marek > > >>>>Date: Mon, 15 Dec 2014 00:31:42 +0100 > > >>>>Subject: [PATCH] cmdutils: dont call read_header before listing > devices > > >>>> > > >>>>List device callback must be able to return valid list without > opening device. > > >>>>This callback should return input values for open function, not > vice-versa. > > >>>>Read header funtion is very likey to fail without proper > configuration provided. > > >>> > > >>>should be ok > > >> > > >>I changed a patch. I wanted to avoid situation where .read_close is > > >>called without .read_header being called before. > > >> > > > > > >> cmdutils.c | 41 +++-- > > >> 1 file changed, 35 insertions(+), 6 deletions(-) > > >>9a93c401d795bae3545a6c6112e71abd98ac22ca > 0001-cmdutils-dont-call-read_header-before-listing-device.patch > > >> From 58fe020b8f1c0e809362e152febe3ad715b1c8fc Mon Sep 17 00:00:00 2001 > > >>From: Lukasz Marek > > >>Date: Mon, 15 Dec 2014 00:31:42 +0100 > > >>Subject: [PATCH] cmdutils: dont call read_header before listing devices > > >> > > >>List device callback must be able to return valid list without opening > device. > > >>This callback should return input values for open function, not > vice-versa. > > >>Read header funtion is very likey to fail without proper configuration > provided. > > >> > > >>Signed-off-by: Lukasz Marek > > >>--- > > >> cmdutils.c | 41 +++-- > > >> 1 file changed, 35 insertions(+), 6 deletions(-) > > >> > > >>diff --git a/cmdutils.c b/cmdutils.c > > >>index 4e0a406..23a5f77 100644 > > >>--- a/cmdutils.c > > >>+++ b/cmdutils.c > > >>@@ -2052,7 +2052,37 @@ void *grow_array(void *array, int elem_size, > int *size, int new_size) > > >> } > > >> > > >> #if CONFIG_AVDEVICE > > >>-static int print_device_sources(AVInputFormat *fmt, AVDictionary > *opts) > > >>+static int alloc_input_context(AVFormatContext **avctx, AVInputFormat > *iformat) > > >>+{ > > >>+AVFormatContext *s = avformat_alloc_context(); > > >>+int ret = 0; > > >>+ > > >>+*avctx = NULL; > > >>+if (!s) > > >>+return AVERROR(ENOMEM); > > >>+ > > >>+s->iformat = iformat; > > >>+if (s->iformat->priv_data_size > 0) { > > >>+s->priv_data = av_mallocz(s->iformat->priv_data_size); > > >>+if (!s->priv_data) { > > >>+ret = AVERROR(ENOMEM); > > >>+goto error; > > >>+} > > >>+if (s->iformat->priv_class) { > > >>+*(const AVClass**)s->priv_data= s->iformat->priv_class; > > >>+av_opt_set_defaults(s->priv_data); > > >>+} > > >>+} else > > >>+s->priv_data = NULL; > > >>+ > > >>+*avctx = s; > > >>+return 0; > > >>+ error: > > >>+avformat_free_context(s); > > >>+return ret; > > >>+} > > >>+ > > > > > >>+static int print_device_sources(void *pfmt, AVDictionary *opts) > > >> { > > >> int ret, i; > > >> AVFormatContext *dev = NULL; > > >>@@ -2069,13 +2099,12 @@ static int print_device_sources(AVInputFormat > *fmt, AVDictionary *opts) > > >> goto fail; > > >> } > > >> > > >>-/* TODO: avformat_open_input calls read_header callback which
Re: [FFmpeg-devel] [PATCH] cmdutils: dont call read_header before listing devices
On 18.12.2014 01:09, Michael Niedermayer wrote: On Wed, Dec 17, 2014 at 10:59:37PM +0100, Lukasz Marek wrote: On 15.12.2014 14:18, Michael Niedermayer wrote: cmdutils.c |8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) 8d012a5193b0440717f89d920661913ef160e674 0001-cmdutils-dont-call-read_header-before-listing-device.patch From 332bb7456c498518ea72dfdaa0e8c3e76d383f21 Mon Sep 17 00:00:00 2001 From: Lukasz Marek Date: Mon, 15 Dec 2014 00:31:42 +0100 Subject: [PATCH] cmdutils: dont call read_header before listing devices List device callback must be able to return valid list without opening device. This callback should return input values for open function, not vice-versa. Read header funtion is very likey to fail without proper configuration provided. should be ok I changed a patch. I wanted to avoid situation where .read_close is called without .read_header being called before. cmdutils.c | 41 +++-- 1 file changed, 35 insertions(+), 6 deletions(-) 9a93c401d795bae3545a6c6112e71abd98ac22ca 0001-cmdutils-dont-call-read_header-before-listing-device.patch From 58fe020b8f1c0e809362e152febe3ad715b1c8fc Mon Sep 17 00:00:00 2001 From: Lukasz Marek Date: Mon, 15 Dec 2014 00:31:42 +0100 Subject: [PATCH] cmdutils: dont call read_header before listing devices List device callback must be able to return valid list without opening device. This callback should return input values for open function, not vice-versa. Read header funtion is very likey to fail without proper configuration provided. Signed-off-by: Lukasz Marek --- cmdutils.c | 41 +++-- 1 file changed, 35 insertions(+), 6 deletions(-) diff --git a/cmdutils.c b/cmdutils.c index 4e0a406..23a5f77 100644 --- a/cmdutils.c +++ b/cmdutils.c @@ -2052,7 +2052,37 @@ void *grow_array(void *array, int elem_size, int *size, int new_size) } #if CONFIG_AVDEVICE -static int print_device_sources(AVInputFormat *fmt, AVDictionary *opts) +static int alloc_input_context(AVFormatContext **avctx, AVInputFormat *iformat) +{ +AVFormatContext *s = avformat_alloc_context(); +int ret = 0; + +*avctx = NULL; +if (!s) +return AVERROR(ENOMEM); + +s->iformat = iformat; +if (s->iformat->priv_data_size > 0) { +s->priv_data = av_mallocz(s->iformat->priv_data_size); +if (!s->priv_data) { +ret = AVERROR(ENOMEM); +goto error; +} +if (s->iformat->priv_class) { +*(const AVClass**)s->priv_data= s->iformat->priv_class; +av_opt_set_defaults(s->priv_data); +} +} else +s->priv_data = NULL; + +*avctx = s; +return 0; + error: +avformat_free_context(s); +return ret; +} + +static int print_device_sources(void *pfmt, AVDictionary *opts) { int ret, i; AVFormatContext *dev = NULL; @@ -2069,13 +2099,12 @@ static int print_device_sources(AVInputFormat *fmt, AVDictionary *opts) goto fail; } -/* TODO: avformat_open_input calls read_header callback which is not necessary. - Function like avformat_alloc_output_context2 for input could be helpful here. */ -av_dict_copy(&tmp_opts, opts, 0); -if ((ret = avformat_open_input(&dev, NULL, fmt, &tmp_opts)) < 0) { +if ((ret = alloc_input_context(&dev, fmt)) < 0) { this fails building due to lack of a fmt variable I failed at cherry-pick conflict, thanks. Updated patch is attached. >From 2b9a20f97687f11eb5d1fd72db3b25e3f2703b73 Mon Sep 17 00:00:00 2001 From: Lukasz Marek Date: Mon, 15 Dec 2014 00:31:42 +0100 Subject: [PATCH] cmdutils: dont call read_header before listing devices List device callback must be able to return valid list without opening device. This callback should return input values for open function, not vice-versa. Read header funtion is very likey to fail without proper configuration provided. Signed-off-by: Lukasz Marek --- cmdutils.c | 39 ++- 1 file changed, 34 insertions(+), 5 deletions(-) diff --git a/cmdutils.c b/cmdutils.c index 4e0a406..eb504a6 100644 --- a/cmdutils.c +++ b/cmdutils.c @@ -2052,6 +2052,36 @@ void *grow_array(void *array, int elem_size, int *size, int new_size) } #if CONFIG_AVDEVICE +static int alloc_input_context(AVFormatContext **avctx, AVInputFormat *iformat) +{ +AVFormatContext *s = avformat_alloc_context(); +int ret = 0; + +*avctx = NULL; +if (!s) +return AVERROR(ENOMEM); + +s->iformat = iformat; +if (s->iformat->priv_data_size > 0) { +s->priv_data = av_mallocz(s->iformat->priv_data_size); +if (!s->priv_data) { +ret = AVERROR(ENOMEM); +goto error; +} +if (s->iformat->priv_class) { +*(const AVClass**)s->priv_data= s->iformat->priv_class; +
Re: [FFmpeg-devel] [PATCH] cmdutils: dont call read_header before listing devices
On 15.12.2014 14:18, Michael Niedermayer wrote: cmdutils.c |8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) 8d012a5193b0440717f89d920661913ef160e674 0001-cmdutils-dont-call-read_header-before-listing-device.patch From 332bb7456c498518ea72dfdaa0e8c3e76d383f21 Mon Sep 17 00:00:00 2001 From: Lukasz Marek Date: Mon, 15 Dec 2014 00:31:42 +0100 Subject: [PATCH] cmdutils: dont call read_header before listing devices List device callback must be able to return valid list without opening device. This callback should return input values for open function, not vice-versa. Read header funtion is very likey to fail without proper configuration provided. should be ok I changed a patch. I wanted to avoid situation where .read_close is called without .read_header being called before. >From 58fe020b8f1c0e809362e152febe3ad715b1c8fc Mon Sep 17 00:00:00 2001 From: Lukasz Marek Date: Mon, 15 Dec 2014 00:31:42 +0100 Subject: [PATCH] cmdutils: dont call read_header before listing devices List device callback must be able to return valid list without opening device. This callback should return input values for open function, not vice-versa. Read header funtion is very likey to fail without proper configuration provided. Signed-off-by: Lukasz Marek --- cmdutils.c | 41 +++-- 1 file changed, 35 insertions(+), 6 deletions(-) diff --git a/cmdutils.c b/cmdutils.c index 4e0a406..23a5f77 100644 --- a/cmdutils.c +++ b/cmdutils.c @@ -2052,7 +2052,37 @@ void *grow_array(void *array, int elem_size, int *size, int new_size) } #if CONFIG_AVDEVICE -static int print_device_sources(AVInputFormat *fmt, AVDictionary *opts) +static int alloc_input_context(AVFormatContext **avctx, AVInputFormat *iformat) +{ +AVFormatContext *s = avformat_alloc_context(); +int ret = 0; + +*avctx = NULL; +if (!s) +return AVERROR(ENOMEM); + +s->iformat = iformat; +if (s->iformat->priv_data_size > 0) { +s->priv_data = av_mallocz(s->iformat->priv_data_size); +if (!s->priv_data) { +ret = AVERROR(ENOMEM); +goto error; +} +if (s->iformat->priv_class) { +*(const AVClass**)s->priv_data= s->iformat->priv_class; +av_opt_set_defaults(s->priv_data); +} +} else +s->priv_data = NULL; + +*avctx = s; +return 0; + error: +avformat_free_context(s); +return ret; +} + +static int print_device_sources(void *pfmt, AVDictionary *opts) { int ret, i; AVFormatContext *dev = NULL; @@ -2069,13 +2099,12 @@ static int print_device_sources(AVInputFormat *fmt, AVDictionary *opts) goto fail; } -/* TODO: avformat_open_input calls read_header callback which is not necessary. - Function like avformat_alloc_output_context2 for input could be helpful here. */ -av_dict_copy(&tmp_opts, opts, 0); -if ((ret = avformat_open_input(&dev, NULL, fmt, &tmp_opts)) < 0) { +if ((ret = alloc_input_context(&dev, fmt)) < 0) { printf("Cannot open device: %s.\n", fmt->name); goto fail; } +av_dict_copy(&tmp_opts, opts, 0); +av_opt_set_dict2(dev, &tmp_opts, AV_OPT_SEARCH_CHILDREN); if ((ret = avdevice_list_devices(dev, &device_list)) < 0) { printf("Cannot list sources.\n"); @@ -2090,7 +2119,7 @@ static int print_device_sources(AVInputFormat *fmt, AVDictionary *opts) fail: av_dict_free(&tmp_opts); avdevice_free_list_devices(&device_list); -avformat_close_input(&dev); +avformat_free_context(dev); return ret; } -- 1.9.1 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] lavd/alsa-audio-common: dont crash while closing not opened device
On 15 December 2014 at 14:05, Nicolas George wrote: > > Le quintidi 25 frimaire, an CCXXIII, Lukasz Marek a écrit : > > for example > > ffmpeg -f alsa -i -f alsa aaa > > would crash if ffmpeg tried to close alsa after open fail. It doesn't so > it > > doesn't occur. > > > > This commit also make safe to call close function twice on the same > context > > In that case, I am not sure if that is such a good idea. Applications > should > not should not or must not? It is a big difference and I'm not sure it is documented anywhere > be calling av_write_trailer() on unopened contexts, it is not a benign > operation like free(). But I will not insist on it. > To be precise I've got crash after applying patch from: https://ffmpeg.org/pipermail/ffmpeg-devel/2014-December/166636.html and using ./ffmpeg -sources alsa The mentioned commit is a bit wrong because it makes avformat_open_input to not call read_header, but close callback is called later inside avformat_close_input. This situation is wrong and I will fix it. Calling close callback without read_header called earlier may be dangerous - right. After that this commit will probably not be needed, but why you think crash, even after wrong? use is not worth to fix? IMHO doxy may states that close in not needed after open fail, but crash is a bit wrong in this case. Also it is not user-friendly design. Error handling section is getting complicated a bit more... ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] lavd/alsa-audio-common: dont crash while closing not opened device
On 15 December 2014 at 13:05, Nicolas George wrote: > > Le quintidi 25 frimaire, an CCXXIII, Lukasz Marek a écrit : > > snd_pcm_close() doesn't handle NULL correctly. > > > > Signed-off-by: Lukasz Marek > > --- > > libavdevice/alsa-audio-common.c | 5 - > > 1 file changed, 4 insertions(+), 1 deletion(-) > > I think it is ok, but can you explain the code path that leads to it? > for example ffmpeg -f alsa -i -f alsa aaa would crash if ffmpeg tried to close alsa after open fail. It doesn't so it doesn't occur. This commit also make safe to call close function twice on the same context ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] lavd/alsa-audio-common: dont crash while closing not opened device
snd_pcm_close() doesn't handle NULL correctly. Signed-off-by: Lukasz Marek --- libavdevice/alsa-audio-common.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/libavdevice/alsa-audio-common.c b/libavdevice/alsa-audio-common.c index b7f5313..fff21a1 100644 --- a/libavdevice/alsa-audio-common.c +++ b/libavdevice/alsa-audio-common.c @@ -303,7 +303,10 @@ av_cold int ff_alsa_close(AVFormatContext *s1) av_freep(&s->reorder_buf); if (CONFIG_ALSA_INDEV) ff_timefilter_destroy(s->timefilter); -snd_pcm_close(s->h); +if (s->h) { +snd_pcm_close(s->h); +s->h = NULL; +} return 0; } -- 1.9.1 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] cmdutils: dont call read_header before listing devices
On 15.12.2014 00:33, Lukasz Marek wrote: List device callback must be able to return valid list without opening device. This callback should return input values for open function, not vice-versa. Read header funtion is very likey to fail without proper configuration provided. Signed-off-by: Lukasz Marek --- cmdutils.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/cmdutils.c b/cmdutils.c index 06ce5d5..51fd777 100644 --- a/cmdutils.c +++ b/cmdutils.c @@ -2069,9 +2069,8 @@ static int print_device_sources(AVInputFormat *fmt, AVDictionary *opts) goto fail; } -/* TODO: avformat_open_input calls read_header callback which is not necessary. - Function like avformat_alloc_output_context2 for input could be helpful here. */ av_dict_copy(&tmp_opts, opts, 0); +dev->flags |= AVFMT_FLAG_PRIV_OPT; if ((ret = avformat_open_input(&dev, NULL, fmt, &tmp_opts)) < 0) { printf("Cannot open device: %s.\n", fmt->name); goto fail; I forgot to amend. Updated patch attached. >From 332bb7456c498518ea72dfdaa0e8c3e76d383f21 Mon Sep 17 00:00:00 2001 From: Lukasz Marek Date: Mon, 15 Dec 2014 00:31:42 +0100 Subject: [PATCH] cmdutils: dont call read_header before listing devices List device callback must be able to return valid list without opening device. This callback should return input values for open function, not vice-versa. Read header funtion is very likey to fail without proper configuration provided. Signed-off-by: Lukasz Marek --- cmdutils.c | 8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/cmdutils.c b/cmdutils.c index 06ce5d5..3e932db 100644 --- a/cmdutils.c +++ b/cmdutils.c @@ -2069,9 +2069,13 @@ static int print_device_sources(AVInputFormat *fmt, AVDictionary *opts) goto fail; } -/* TODO: avformat_open_input calls read_header callback which is not necessary. - Function like avformat_alloc_output_context2 for input could be helpful here. */ av_dict_copy(&tmp_opts, opts, 0); +dev = avformat_alloc_context(); +if (!dev) { +ret = AVERROR(ENOMEM); +goto fail; +} +dev->flags |= AVFMT_FLAG_PRIV_OPT; if ((ret = avformat_open_input(&dev, NULL, fmt, &tmp_opts)) < 0) { printf("Cannot open device: %s.\n", fmt->name); goto fail; -- 1.9.1 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] cmdutils: dont call read_header before listing devices
List device callback must be able to return valid list without opening device. This callback should return input values for open function, not vice-versa. Read header funtion is very likey to fail without proper configuration provided. Signed-off-by: Lukasz Marek --- cmdutils.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/cmdutils.c b/cmdutils.c index 06ce5d5..51fd777 100644 --- a/cmdutils.c +++ b/cmdutils.c @@ -2069,9 +2069,8 @@ static int print_device_sources(AVInputFormat *fmt, AVDictionary *opts) goto fail; } -/* TODO: avformat_open_input calls read_header callback which is not necessary. - Function like avformat_alloc_output_context2 for input could be helpful here. */ av_dict_copy(&tmp_opts, opts, 0); +dev->flags |= AVFMT_FLAG_PRIV_OPT; if ((ret = avformat_open_input(&dev, NULL, fmt, &tmp_opts)) < 0) { printf("Cannot open device: %s.\n", fmt->name); goto fail; -- 1.9.1 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 2/2] lavd/avdevice: use better option types for caps options
On 13.12.2014 23:11, Michael Niedermayer wrote: On Sat, Dec 13, 2014 at 08:27:08PM +0100, Lukasz Marek wrote: Using dedicated types allows to use format/layout names, not just raw int values. Signed-off-by: Lukasz Marek LGTM pushed this one. first from patchset dropped. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel