Re: [FFmpeg-devel] [PATCH] avfilter/vidstab: add option for file format specification

[Gyan Doshi <ffmpeg@gyani.pro>]

On 2023-10-22 05:36 pm, Timo Rothenpieler wrote:
> On 22.10.2023 13:57, Gyan Doshi wrote:
>>
>>
>> On 2023-10-22 05:04 pm, Timo Rothenpieler wrote:
>>> On 22.10.2023 13:24, Gyan Doshi wrote:
>>>> The vidstab library added support in Nov 2020 for writing/reading
>>>> the transforms data in binary in addition to ASCII. The library 
>>>> default
>>>> was changed to binary format but no changes were made to the AVfilters
>>>> resulting in data file for writing or reading being always opened 
>>>> as text.
>>>> This effectively broke the filters.
>>>>
>>>> Options added to vidstab{detect,transform} to specify file format
>>>> and open files with the correct attributes.
>>>> ---
>>>>   doc/filters.texi                  | 26 ++++++++++++++++++++++++++
>>>>   libavfilter/vf_vidstabdetect.c    | 15 ++++++++++++++-
>>>>   libavfilter/vf_vidstabtransform.c | 15 ++++++++++++++-
>>>>   3 files changed, 54 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/doc/filters.texi b/doc/filters.texi
>>>> index f5032ddf74..806448f063 100644
>>>> --- a/doc/filters.texi
>>>> +++ b/doc/filters.texi
>>>> @@ -24618,6 +24618,19 @@ If set to 0, it is disabled. The frames 
>>>> are counted starting from 1.
>>>>   Show fields and transforms in the resulting frames. It accepts an
>>>>   integer in the range 0-2. Default value is 0, which disables any
>>>>   visualization.
>>>> +
>>>> +@item fileformat
>>>> +Format for the transforms data file to be written.
>>>> +Acceptable values are
>>>> +
>>>> +@table @samp
>>>> +@item ascii
>>>> +Human-readable plain text
>>>> +
>>>> +@item binary
>>>> +Binary format, roughly 40% smaller than @code{ascii}. 
>>>> (@emph{default})
>>>> +@end table
>>>> +
>>>>   @end table
>>>>     @subsection Examples
>>>> @@ -24772,6 +24785,19 @@ Use also @code{tripod} option of 
>>>> @ref{vidstabdetect}.
>>>>   Increase log verbosity if set to 1. Also the detected global motions
>>>>   are written to the temporary file @file{global_motions.trf}. Default
>>>>   value is 0.
>>>> +
>>>> +@item fileformat
>>>> +Format of the transforms data file to be read.
>>>> +Acceptable values are
>>>> +
>>>> +@table @samp
>>>> +@item ascii
>>>> +Human-readable plain text
>>>> +
>>>> +@item binary
>>>> +Binary format (@emph{default})
>>>> +@end table
>>>> +
>>>>   @end table
>>>>     @subsection Examples
>>>> diff --git a/libavfilter/vf_vidstabdetect.c 
>>>> b/libavfilter/vf_vidstabdetect.c
>>>> index a2c6d89503..aa050afab9 100644
>>>> --- a/libavfilter/vf_vidstabdetect.c
>>>> +++ b/libavfilter/vf_vidstabdetect.c
>>>> @@ -40,6 +40,7 @@ typedef struct StabData {
>>>>       VSMotionDetectConfig conf;
>>>>         char *result;
>>>> +    int fileformat;
>>>>       FILE *f;
>>>>   } StabData;
>>>>   @@ -58,6 +59,11 @@ static const AVOption vidstabdetect_options[] = {
>>>>       {"show",        "0: draw nothing; 1,2: show fields and 
>>>> transforms",              OFFSETC(show), AV_OPT_TYPE_INT, {.i64 = 
>>>> 0},      0,   2, FLAGS},
>>>>       {"tripod",      "virtual tripod mode (if >0): motion is 
>>>> compared to a reference"
>>>>                       " reference frame (frame # is the 
>>>> value)",                       OFFSETC(virtualTripod), 
>>>> AV_OPT_TYPE_INT,    {.i64 = 0}, 0, INT_MAX, FLAGS},
>>>> +#ifdef LIBVIDSTAB_FILE_FORMAT_VERSION
>>>> +    { "fileformat",   "transforms data file format", 
>>>> OFFSET(fileformat),  AV_OPT_TYPE_INT,  {.i64 = 
>>>> BINARY_SERIALIZATION_MODE},  ASCII_SERIALIZATION_MODE, 
>>>> BINARY_SERIALIZATION_MODE,  FLAGS,  "file_format"},
>>>> +    { "ascii",        "ASCII text",  0,  AV_OPT_TYPE_CONST, {.i64 
>>>> = ASCII_SERIALIZATION_MODE },  0,  0,  FLAGS, "file_format"},
>>>> +    { "binary",       "binary",      0,  AV_OPT_TYPE_CONST, {.i64 
>>>> = BINARY_SERIALIZATION_MODE},  0,  0,  FLAGS, "file_format"},
>>>> +#endif
>>>>       {NULL}
>>>>   };
>>>>   @@ -94,6 +100,13 @@ static int config_input(AVFilterLink *inlink)
>>>>       VSFrameInfo fi;
>>>>       const AVPixFmtDescriptor *desc = 
>>>> av_pix_fmt_desc_get(inlink->format);
>>>>       int is_planar = desc->flags & AV_PIX_FMT_FLAG_PLANAR;
>>>> +    const char *file_mode = "w";
>>>> +
>>>> +#ifdef LIBVIDSTAB_FILE_FORMAT_VERSION
>>>> +    md->serializationMode = s->fileformat;
>>>> +    if (s->fileformat == BINARY_SERIALIZATION_MODE)
>>>> +        file_mode = "wb";
>>>> +#endif
>>>>         vsFrameInfoInit(&fi, inlink->w, inlink->h,
>>>>                       ff_av2vs_pixfmt(ctx, inlink->format));
>>>> @@ -129,7 +142,7 @@ static int config_input(AVFilterLink *inlink)
>>>>       av_log(ctx, AV_LOG_INFO, "          show = %d\n", s->conf.show);
>>>>       av_log(ctx, AV_LOG_INFO, "        result = %s\n", s->result);
>>>>   -    s->f = avpriv_fopen_utf8(s->result, "w");
>>>> +    s->f = avpriv_fopen_utf8(s->result, file_mode);
>>>>       if (s->f == NULL) {
>>>>           av_log(ctx, AV_LOG_ERROR, "cannot open transform file 
>>>> %s\n", s->result);
>>>>           return AVERROR(EINVAL);
>>>> diff --git a/libavfilter/vf_vidstabtransform.c 
>>>> b/libavfilter/vf_vidstabtransform.c
>>>> index 8a66a463b4..780bf1064d 100644
>>>> --- a/libavfilter/vf_vidstabtransform.c
>>>> +++ b/libavfilter/vf_vidstabtransform.c
>>>> @@ -42,6 +42,7 @@ typedef struct TransformContext {
>>>>       char *input;                // name of transform file
>>>>       int tripod;
>>>>       int debug;
>>>> +    int fileformat;
>>>>   } TransformContext;
>>>>     #define OFFSET(x) offsetof(TransformContext, x)
>>>> @@ -101,6 +102,12 @@ static const AVOption 
>>>> vidstabtransform_options[] = {
>>>>                      AV_OPT_TYPE_BOOL,   {.i64 = 0}, 0, 1, FLAGS},
>>>>       {"debug",     "enable debug mode and writer global motions 
>>>> information to file", OFFSET(debug),
>>>>                      AV_OPT_TYPE_BOOL,   {.i64 = 0}, 0, 1, FLAGS},
>>>> +#ifdef LIBVIDSTAB_FILE_FORMAT_VERSION
>>>> +    { "fileformat",   "transforms data file format", 
>>>> OFFSET(fileformat),
>>>> +                   AV_OPT_TYPE_INT,  {.i64 = 
>>>> BINARY_SERIALIZATION_MODE},  ASCII_SERIALIZATION_MODE, 
>>>> BINARY_SERIALIZATION_MODE,  FLAGS,  "file_format"},
>>>> +    { "ascii",        "ASCII text",  0,  AV_OPT_TYPE_CONST, {.i64 
>>>> = ASCII_SERIALIZATION_MODE },  0,  0,  FLAGS, "file_format"},
>>>> +    { "binary",       "binary",      0,  AV_OPT_TYPE_CONST, {.i64 
>>>> = BINARY_SERIALIZATION_MODE},  0,  0,  FLAGS, "file_format"},
>>>> +#endif
>>>>       {NULL}
>>>>   };
>>>>   @@ -131,6 +138,12 @@ static int config_input(AVFilterLink *inlink)
>>>>         const AVPixFmtDescriptor *desc = 
>>>> av_pix_fmt_desc_get(inlink->format);
>>>>       int is_planar = desc->flags & AV_PIX_FMT_FLAG_PLANAR;
>>>> +    const char *file_mode = "r";
>>>> +
>>>> +#ifdef LIBVIDSTAB_FILE_FORMAT_VERSION
>>>> +    if (tc->fileformat == BINARY_SERIALIZATION_MODE)
>>>> +        file_mode = "rb";
>>>> +#endif
>>>>         VSTransformData *td = &(tc->td);
>>>>   @@ -193,7 +206,7 @@ static int config_input(AVFilterLink *inlink)
>>>>           av_log(ctx, AV_LOG_INFO, "    zoomspeed = %g\n", 
>>>> tc->conf.zoomSpeed);
>>>>       av_log(ctx, AV_LOG_INFO, "    interpol  = %s\n", 
>>>> getInterpolationTypeName(tc->conf.interpolType));
>>>>   -    f = avpriv_fopen_utf8(tc->input, "r");
>>>> +    f = avpriv_fopen_utf8(tc->input, file_mode);
>>>
>>> Can't it just always open it in binary mode, and work fine no matter 
>>> what format it is?
>>> The library does have auto-detection logic in place after all.
>>>
>>> The user having to know the format and passing it as an option to 
>>> the filter seems a bit annoying.
>>
>> Fine by me. Sending revised patch.
>
> It was an actual question, I'm not sure if it works reliably.
> Specially on Windows where line ending conversion stuff might be going 
> on.
>
> Which makes me wonder if it shouldn't also always open for writing in 
> binary mode.

The 1st stage filter has been writing in text mode since its inception. 
And prior to the format addition, I haven't had any complaints from 
Windows users.
So, the writing part doesn't need to be forced to binary.

Regards,
Gyan

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".