diff mbox

[FFmpeg-devel,PATCHv2,8/8] avformat/img2enc: add support for specifying protocol options

Message ID 20191228114635.7727-1-cus@passwd.hu
State New
Headers show

Commit Message

Marton Balint Dec. 28, 2019, 11:46 a.m. UTC
v2: simplified example

Signed-off-by: Marton Balint <cus@passwd.hu>
---
 doc/muxers.texi       | 11 +++++++++++
 libavformat/img2enc.c | 13 ++++++++++++-
 2 files changed, 23 insertions(+), 1 deletion(-)

Comments

Marton Balint Jan. 1, 2020, 7:10 p.m. UTC | #1
On Tue, 31 Dec 2019, Michael Niedermayer wrote:

> On Tue, Dec 31, 2019 at 12:37:02PM +0100, Nicolas George wrote:
>> Marton Balint (12019-12-28):
>>> v2: simplified example
>>>
>>> Signed-off-by: Marton Balint <cus@passwd.hu>
>>> ---
>>>  doc/muxers.texi       | 11 +++++++++++
>>>  libavformat/img2enc.c | 13 ++++++++++++-
>>>  2 files changed, 23 insertions(+), 1 deletion(-)
>>
>> image2 is not the only demuxer that opens new streams. I think a generic
>> solution would be preferable.
>
> i also had a slightly ungood feeling about the patch but didnt
> had time to think more about it. a more generic solution like with
> child AVClasses or something would be interresting but as said i didnt
> had time to think about this ...

It looks like a big can of worms.

In the AVFMT_NOFILE case there is no IO context, so options can only be 
passed using avformat_write_header/avformat_init_output.

There is no way to determine which options the protocols will use 
without actually opening an IO context (protocol options are passed to the 
url_open method of each protocol), so we have to store all remaining 
options passed to avformat_write_header/avformat_init_output for possible 
nested IO use.

In the normal case (non AVFMT_NOFILE) muxers can use nested contexts too, 
so avio_open should also store the original options, all of them, because 
the nested contexts might use different protocols. This alone is 
problematic, because avio_open should return the unrecognized options...

Also it might make sense to specify different IO options for nested 
contexts than main contexts (or different options for some of the nested 
contexts)

IMHO being able to specify the nested options separately is a 
clean solution, admittedly less user friendly, but I don't see how this 
can work automagically without some major redesign.

Regards,
Marton
Marton Balint Jan. 8, 2020, 10:49 p.m. UTC | #2
On Wed, 1 Jan 2020, Marton Balint wrote:

>
> On Tue, 31 Dec 2019, Michael Niedermayer wrote:
>
>> On Tue, Dec 31, 2019 at 12:37:02PM +0100, Nicolas George wrote:
>>> Marton Balint (12019-12-28):
>>>> v2: simplified example
>>>>
>>>> Signed-off-by: Marton Balint <cus@passwd.hu>
>>>> ---
>>>>  doc/muxers.texi       | 11 +++++++++++
>>>>  libavformat/img2enc.c | 13 ++++++++++++-
>>>>  2 files changed, 23 insertions(+), 1 deletion(-)
>>>
>>> image2 is not the only demuxer that opens new streams. I think a generic
>>> solution would be preferable.
>>
>> i also had a slightly ungood feeling about the patch but didnt
>> had time to think more about it. a more generic solution like with
>> child AVClasses or something would be interresting but as said i didnt
>> had time to think about this ...
>
> It looks like a big can of worms.
>
> In the AVFMT_NOFILE case there is no IO context, so options can only be 
> passed using avformat_write_header/avformat_init_output.
>
> There is no way to determine which options the protocols will use 
> without actually opening an IO context (protocol options are passed to the 
> url_open method of each protocol), so we have to store all remaining 
> options passed to avformat_write_header/avformat_init_output for possible 
> nested IO use.
>
> In the normal case (non AVFMT_NOFILE) muxers can use nested contexts too, 
> so avio_open should also store the original options, all of them, because 
> the nested contexts might use different protocols. This alone is 
> problematic, because avio_open should return the unrecognized options...
>
> Also it might make sense to specify different IO options for nested 
> contexts than main contexts (or different options for some of the nested 
> contexts)
>
> IMHO being able to specify the nested options separately is a 
> clean solution, admittedly less user friendly, but I don't see how this 
> can work automagically without some major redesign.

Ping for this.

Thanks,
Marton
Michael Niedermayer Jan. 9, 2020, 7:07 p.m. UTC | #3
On Wed, Jan 01, 2020 at 08:10:06PM +0100, Marton Balint wrote:
> 
> 
> On Tue, 31 Dec 2019, Michael Niedermayer wrote:
> 
> >On Tue, Dec 31, 2019 at 12:37:02PM +0100, Nicolas George wrote:
> >>Marton Balint (12019-12-28):
> >>>v2: simplified example
> >>>
> >>>Signed-off-by: Marton Balint <cus@passwd.hu>
> >>>---
> >>> doc/muxers.texi       | 11 +++++++++++
> >>> libavformat/img2enc.c | 13 ++++++++++++-
> >>> 2 files changed, 23 insertions(+), 1 deletion(-)
> >>
> >>image2 is not the only demuxer that opens new streams. I think a generic
> >>solution would be preferable.
> >
> >i also had a slightly ungood feeling about the patch but didnt
> >had time to think more about it. a more generic solution like with
> >child AVClasses or something would be interresting but as said i didnt
> >had time to think about this ...
> 
> It looks like a big can of worms.
> 
> In the AVFMT_NOFILE case there is no IO context, so options can only be
> passed using avformat_write_header/avformat_init_output.
> 
> There is no way to determine which options the protocols will use without
> actually opening an IO context (protocol options are passed to the url_open
> method of each protocol), so we have to store all remaining options passed
> to avformat_write_header/avformat_init_output for possible nested IO use.
> 
> In the normal case (non AVFMT_NOFILE) muxers can use nested contexts too, so
> avio_open should also store the original options, all of them, because the
> nested contexts might use different protocols. This alone is problematic,
> because avio_open should return the unrecognized options...
> 
> Also it might make sense to specify different IO options for nested contexts
> than main contexts (or different options for some of the nested contexts)
> 
> IMHO being able to specify the nested options separately is a clean
> solution, admittedly less user friendly, but I don't see how this can work
> automagically without some major redesign.

ok, yes i agree it would need a too major redesign.

thx

[...]
Marton Balint Jan. 15, 2020, 6:37 p.m. UTC | #4
On Thu, 9 Jan 2020, Michael Niedermayer wrote:

> On Wed, Jan 01, 2020 at 08:10:06PM +0100, Marton Balint wrote:
>>
>>
>> On Tue, 31 Dec 2019, Michael Niedermayer wrote:
>>
>>> On Tue, Dec 31, 2019 at 12:37:02PM +0100, Nicolas George wrote:
>>>> Marton Balint (12019-12-28):
>>>>> v2: simplified example
>>>>>
>>>>> Signed-off-by: Marton Balint <cus@passwd.hu>
>>>>> ---
>>>>> doc/muxers.texi       | 11 +++++++++++
>>>>> libavformat/img2enc.c | 13 ++++++++++++-
>>>>> 2 files changed, 23 insertions(+), 1 deletion(-)
>>>>
>>>> image2 is not the only demuxer that opens new streams. I think a generic
>>>> solution would be preferable.
>>>
>>> i also had a slightly ungood feeling about the patch but didnt
>>> had time to think more about it. a more generic solution like with
>>> child AVClasses or something would be interresting but as said i didnt
>>> had time to think about this ...
>>
>> It looks like a big can of worms.
>>
>> In the AVFMT_NOFILE case there is no IO context, so options can only be
>> passed using avformat_write_header/avformat_init_output.
>>
>> There is no way to determine which options the protocols will use without
>> actually opening an IO context (protocol options are passed to the url_open
>> method of each protocol), so we have to store all remaining options passed
>> to avformat_write_header/avformat_init_output for possible nested IO use.
>>
>> In the normal case (non AVFMT_NOFILE) muxers can use nested contexts too, so
>> avio_open should also store the original options, all of them, because the
>> nested contexts might use different protocols. This alone is problematic,
>> because avio_open should return the unrecognized options...
>>
>> Also it might make sense to specify different IO options for nested contexts
>> than main contexts (or different options for some of the nested contexts)
>>
>> IMHO being able to specify the nested options separately is a clean
>> solution, admittedly less user friendly, but I don't see how this can work
>> automagically without some major redesign.
>
> ok, yes i agree it would need a too major redesign.

Ok, I will apply in a few days if I receive no further comments.

Thanks,
Marton
Marton Balint Jan. 18, 2020, 11:04 p.m. UTC | #5
On Wed, 15 Jan 2020, Marton Balint wrote:

>
>
> On Thu, 9 Jan 2020, Michael Niedermayer wrote:
>
>> On Wed, Jan 01, 2020 at 08:10:06PM +0100, Marton Balint wrote:
>>>
>>>
>>> On Tue, 31 Dec 2019, Michael Niedermayer wrote:
>>>
>>>> On Tue, Dec 31, 2019 at 12:37:02PM +0100, Nicolas George wrote:
>>>>> Marton Balint (12019-12-28):
>>>>>> v2: simplified example
>>>>>>
>>>>>> Signed-off-by: Marton Balint <cus@passwd.hu>
>>>>>> ---
>>>>>> doc/muxers.texi       | 11 +++++++++++
>>>>>> libavformat/img2enc.c | 13 ++++++++++++-
>>>>>> 2 files changed, 23 insertions(+), 1 deletion(-)
>>>>>
>>>>> image2 is not the only demuxer that opens new streams. I think a generic
>>>>> solution would be preferable.
>>>>
>>>> i also had a slightly ungood feeling about the patch but didnt
>>>> had time to think more about it. a more generic solution like with
>>>> child AVClasses or something would be interresting but as said i didnt
>>>> had time to think about this ...
>>>
>>> It looks like a big can of worms.
>>>
>>> In the AVFMT_NOFILE case there is no IO context, so options can only be
>>> passed using avformat_write_header/avformat_init_output.
>>>
>>> There is no way to determine which options the protocols will use without
>>> actually opening an IO context (protocol options are passed to the 
> url_open
>>> method of each protocol), so we have to store all remaining options passed
>>> to avformat_write_header/avformat_init_output for possible nested IO use.
>>>
>>> In the normal case (non AVFMT_NOFILE) muxers can use nested contexts too, 
> so
>>> avio_open should also store the original options, all of them, because the
>>> nested contexts might use different protocols. This alone is problematic,
>>> because avio_open should return the unrecognized options...
>>>
>>> Also it might make sense to specify different IO options for nested 
> contexts
>>> than main contexts (or different options for some of the nested contexts)
>>>
>>> IMHO being able to specify the nested options separately is a clean
>>> solution, admittedly less user friendly, but I don't see how this can work
>>> automagically without some major redesign.
>>
>> ok, yes i agree it would need a too major redesign.
>
> Ok, I will apply in a few days if I receive no further comments.

Applied.

Thanks,
Marton
diff mbox

Patch

diff --git a/doc/muxers.texi b/doc/muxers.texi
index fb5c9bc4c0..470a11af95 100644
--- a/doc/muxers.texi
+++ b/doc/muxers.texi
@@ -1193,6 +1193,11 @@  overwritten with new images. Default value is 0.
 @item strftime
 If set to 1, expand the filename with date and time information from
 @code{strftime()}. Default value is 0.
+
+@item protocol_opts @var{options_list}
+Set protocol options as a :-separated list of key=value parameters. Values
+containing the @code{:} special character must be escaped.
+
 @end table
 
 @subsection Examples
@@ -1235,6 +1240,12 @@  You can set the file name with current frame's PTS:
 ffmpeg -f v4l2 -r 1 -i /dev/video0 -copyts -f image2 -frame_pts true %d.jpg"
 @end example
 
+A more complex example is to publish contents of your desktop directly to a
+WebDAV server every second:
+@example
+ffmpeg -f x11grab -framerate 1 -i :0.0 -q:v 6 -update 1 -protocol_opts method=PUT http://example.com/desktop.jpg
+@end example
+
 @section matroska
 
 Matroska container muxer.
diff --git a/libavformat/img2enc.c b/libavformat/img2enc.c
index 39398f37a3..0ad4ee11d7 100644
--- a/libavformat/img2enc.c
+++ b/libavformat/img2enc.c
@@ -23,6 +23,7 @@ 
 #include "libavutil/intreadwrite.h"
 #include "libavutil/avassert.h"
 #include "libavutil/avstring.h"
+#include "libavutil/dict.h"
 #include "libavutil/log.h"
 #include "libavutil/opt.h"
 #include "libavutil/pixdesc.h"
@@ -44,6 +45,7 @@  typedef struct VideoMuxData {
     int frame_pts;
     const char *muxer;
     int use_rename;
+    AVDictionary *protocol_opts;
 } VideoMuxData;
 
 static int write_header(AVFormatContext *s)
@@ -133,6 +135,7 @@  static int write_packet(AVFormatContext *s, AVPacket *pkt)
     const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(par->format);
     int ret, i;
     int nb_renames = 0;
+    AVDictionary *options = NULL;
 
     if (img->update) {
         av_strlcpy(filename, img->path, sizeof(filename));
@@ -161,13 +164,19 @@  static int write_packet(AVFormatContext *s, AVPacket *pkt)
         return AVERROR(EINVAL);
     }
     for (i = 0; i < 4; i++) {
+        av_dict_copy(&options, img->protocol_opts, 0);
         snprintf(img->tmp[i], sizeof(img->tmp[i]), "%s.tmp", filename);
         av_strlcpy(img->target[i], filename, sizeof(img->target[i]));
-        if (s->io_open(s, &pb[i], img->use_rename ? img->tmp[i] : filename, AVIO_FLAG_WRITE, NULL) < 0) {
+        if (s->io_open(s, &pb[i], img->use_rename ? img->tmp[i] : filename, AVIO_FLAG_WRITE, &options) < 0) {
             av_log(s, AV_LOG_ERROR, "Could not open file : %s\n", img->use_rename ? img->tmp[i] : filename);
             ret = AVERROR(EIO);
             goto fail;
         }
+        if (options) {
+            av_log(s, AV_LOG_ERROR, "Could not recognize some protocol options\n");
+            ret = AVERROR(EINVAL);
+            goto fail;
+        }
 
         if (!img->split_planes || i+1 >= desc->nb_components)
             break;
@@ -211,6 +220,7 @@  static int write_packet(AVFormatContext *s, AVPacket *pkt)
     return 0;
 
 fail:
+    av_dict_free(&options);
     for (i = 0; i < FF_ARRAY_ELEMS(pb); i++)
         if (pb[i])
             ff_format_io_close(s, &pb[i]);
@@ -236,6 +246,7 @@  static const AVOption muxoptions[] = {
     { "strftime",     "use strftime for filename", OFFSET(use_strftime),  AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, ENC },
     { "frame_pts",    "use current frame pts for filename", OFFSET(frame_pts),  AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, ENC },
     { "atomic_writing", "write files atomically (using temporary files and renames)", OFFSET(use_rename), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, ENC },
+    { "protocol_opts", "specify protocol options for the opened files", OFFSET(protocol_opts), AV_OPT_TYPE_DICT, {0}, 0, 0, ENC },
     { NULL },
 };