diff mbox series

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

Message ID 20231022112446.306-1-ffmpeg@gyani.pro
State New
Headers show
Series [FFmpeg-devel] avfilter/vidstab: add option for file format specification | expand

Checks

Context Check Description
yinshiyou/make_loongarch64 success Make finished
yinshiyou/make_fate_loongarch64 success Make fate finished
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished

Commit Message

Gyan Doshi Oct. 22, 2023, 11:24 a.m. UTC
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(-)

Comments

Timo Rothenpieler Oct. 22, 2023, 11:34 a.m. UTC | #1
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.

>       if (!f) {
>           int ret = AVERROR(errno);
>           av_log(ctx, AV_LOG_ERROR, "cannot open input file %s\n", tc->input);
Gyan Doshi Oct. 22, 2023, 11:57 a.m. UTC | #2
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
Timo Rothenpieler Oct. 22, 2023, 12:06 p.m. UTC | #3
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.

> Although default was binary in the 2nd stage so it worked out of the box 
> without user input.
> 
> Regards,
> Gyan
Gyan Doshi Oct. 22, 2023, 12:17 p.m. UTC | #4
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
diff mbox series

Patch

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);
     if (!f) {
         int ret = AVERROR(errno);
         av_log(ctx, AV_LOG_ERROR, "cannot open input file %s\n", tc->input);