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

[Gyan Doshi <ffmpeg@gyani.pro>]

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.

Although default was binary in the 2nd stage so it worked out of the box 
without user input.

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".