Re: [FFmpeg-devel] subtitles filter and -ss
Thanks for the prompt response. It did save me from wasting more time trying to get the value of -ss starting from the AVFilterContext pointer. I've submitted a patch that adds a "shift" option to the plugin. I've opted for "shift" instead of "seek" as the name of the new option. This should make it evident that the option can also be used for applying minor timing corrections to the subtitles. Any feedback is welcome. Best regards, Manolis On Sun, 3 May 2020 at 17:34, Gyan Doshi wrote: > > > On 03-05-2020 08:36 pm, Manolis Stamatogiannakis wrote: > > Hello, > > > > I've noticed what appears to be a bug/missing feature in the subtitles > > filter: when "-ss" is used for the input, it is not applied to the > > subtitles stream. E.g., for the following command line, the video > playback > > will start on 20:10, but the subtitles will start from 00:00. > > > > ffmpeg -re -ss 20:10.00 -i input.mp4 > > -vf subtitles=subs.srt \ > > -c:v libx264 crf 25 -c:a aac -ab 160k \ > > -strict experimental \ > > -f flv $RTMP_URL > > > > This should be fairly straightforward to fix and review, so I'd like to > > directly submit a patch rather than opening a bug and having someone else > > fix it. As I'm not an expert with the ffmpeg internals, I'd like some > > guidance on implementing a proper fix. > > > > What would be considered more appropriate: > > (a) automatically skipping the subtitles for the time specified by -ss? > > (b) adding a shift option to the subtitles filter that allows shifting > > subtitles time by specific time? > > (c) something else I can't think of? > > -ss executes a seek for that input. The subtitles filter does not have > access to that data. You can add an option to the subtitles filter that > also carries out a seek. And make sure to adjust timestamps depending on > the seek match. > > 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-requ...@ffmpeg.org with subject "unsubscribe". ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH] avfilter/vf_subtitles: add shift options
Allows shifting of subtitle display times to align them with the video.
This avoids having to rewrite the subtitle file in order to display
subtitles correctly when input is seeked (-ss).
Also handy for minor subtitle timing corrections without rewriting the
subtitles file.
Signed-off-by: Manolis Stamatogiannakis
---
doc/filters.texi | 8
libavfilter/vf_subtitles.c | 29 +++--
2 files changed, 35 insertions(+), 2 deletions(-)
diff --git a/doc/filters.texi b/doc/filters.texi
index d19fd346ae..94323495f0 100644
--- a/doc/filters.texi
+++ b/doc/filters.texi
@@ -17851,6 +17851,9 @@ The filter accepts the following options:
@item filename, f
Set the filename of the subtitle file to read. It must be specified.
+@item shift
+Shift subtitles timings by the specified amount.
+
@item original_size
Specify the size of the original video, the video for which the ASS file
was composed. For the syntax of this option, check the
@@ -17907,6 +17910,11 @@ To make the subtitles stream from @file{sub.srt}
appear in 80% transparent blue
subtitles=sub.srt:force_style='FontName=DejaVu Serif,PrimaryColour='
@end example
+To re-sync subtitles after seeking the input e.g. with @code{-ss 20:20}, use:
+@example
+subtitles=filename=sub.srt:shift=-20\:20
+@end example
+
@section super2xsai
Scale the input by 2x and smooth using the Super2xSaI (Scale and
diff --git a/libavfilter/vf_subtitles.c b/libavfilter/vf_subtitles.c
index a3b4029af4..74a902941a 100644
--- a/libavfilter/vf_subtitles.c
+++ b/libavfilter/vf_subtitles.c
@@ -52,6 +52,8 @@ typedef struct AssContext {
char *filename;
char *fontsdir;
char *charenc;
+int64_t shift;
+char *shift_opt;
char *force_style;
int stream_index;
int alpha;
@@ -68,6 +70,7 @@ typedef struct AssContext {
#define COMMON_OPTIONS \
{"filename", "set the filename of file to read",
OFFSET(filename), AV_OPT_TYPE_STRING, {.str = NULL}, 0, 0, FLAGS }, \
{"f", "set the filename of file to read",
OFFSET(filename), AV_OPT_TYPE_STRING, {.str = NULL}, 0, 0, FLAGS }, \
+{"shift", "set the tilename of file to read",
OFFSET(shift_opt), AV_OPT_TYPE_STRING, {.str = NULL}, 0, 0, FLAGS }, \
{"original_size", "set the size of the original video (used to scale
fonts)", OFFSET(original_w), AV_OPT_TYPE_IMAGE_SIZE, {.str = NULL}, 0, 0,
FLAGS }, \
{"fontsdir", "set the directory containing the fonts to read",
OFFSET(fontsdir), AV_OPT_TYPE_STRING, {.str = NULL}, 0, 0, FLAGS }, \
{"alpha", "enable processing of alpha channel",
OFFSET(alpha), AV_OPT_TYPE_BOOL, {.i64 = 0 }, 0,
1, FLAGS }, \
@@ -103,6 +106,16 @@ static av_cold int init(AVFilterContext *ctx)
return AVERROR(EINVAL);
}
+if (ass->shift_opt) {
+if (av_parse_time(>shift, ass->shift_opt, 1) < 0) {
+av_log(ctx, AV_LOG_ERROR, "Invalid subtitles shift: %s\n",
+ ass->shift_opt);
+return AVERROR(EINVAL);
+}
+ass->shift = av_rescale_q(ass->shift, AV_TIME_BASE_Q, av_make_q(1,
1000));
+av_log(ctx, AV_LOG_DEBUG, "Shifting subtitles by %0.3fsec.\n",
ass->shift/1000.0);
+}
+
ass->library = ass_library_init();
if (!ass->library) {
av_log(ctx, AV_LOG_ERROR, "Could not initialize libass.\n");
@@ -297,7 +310,7 @@ AVFILTER_DEFINE_CLASS(subtitles);
static av_cold int init_subtitles(AVFilterContext *ctx)
{
-int j, ret, sid;
+int j, ret, sid, nskip;
int k = 0;
AVDictionary *codec_opts = NULL;
AVFormatContext *fmt = NULL;
@@ -448,6 +461,7 @@ static av_cold int init_subtitles(AVFilterContext *ctx)
av_init_packet();
pkt.data = NULL;
pkt.size = 0;
+nskip = 0;
while (av_read_frame(fmt, ) >= 0) {
int i, got_subtitle;
AVSubtitle sub = {0};
@@ -458,8 +472,17 @@ static av_cold int init_subtitles(AVFilterContext *ctx)
av_log(ctx, AV_LOG_WARNING, "Error decoding: %s (ignored)\n",
av_err2str(ret));
} else if (got_subtitle) {
-const int64_t start_time = av_rescale_q(sub.pts,
AV_TIME_BASE_Q, av_make_q(1, 1000));
+const int64_t start_time = av_rescale_q(sub.pts,
AV_TIME_BASE_Q, av_make_q(1, 1000)) + ass->shift;
const int64_t duration = sub.end_display_time;
+
+if (start_time + duration < 0) {
+nskip++;
+goto pkt_end;
+} else if (nskip > 0) {
+av_log(ctx, AV_LOG_INFO, "Skipped %
[FFmpeg-devel] subtitles filter and -ss
Hello, I've noticed what appears to be a bug/missing feature in the subtitles filter: when "-ss" is used for the input, it is not applied to the subtitles stream. E.g., for the following command line, the video playback will start on 20:10, but the subtitles will start from 00:00. ffmpeg -re -ss 20:10.00 -i input.mp4 -vf subtitles=subs.srt \ -c:v libx264 crf 25 -c:a aac -ab 160k \ -strict experimental \ -f flv $RTMP_URL This should be fairly straightforward to fix and review, so I'd like to directly submit a patch rather than opening a bug and having someone else fix it. As I'm not an expert with the ffmpeg internals, I'd like some guidance on implementing a proper fix. What would be considered more appropriate: (a) automatically skipping the subtitles for the time specified by -ss? (b) adding a shift option to the subtitles filter that allows shifting subtitles time by specific time? (c) something else I can't think of? Although (a) would probably be a bit more elegant to use, I'm not sure if it always makes sense as ffmpeg may use several input files. Thanks, Manolis ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avfilter/vf_subtitles: add shift option
Bumping this up after 10 days without a comment.
On Fri, 8 May 2020 at 13:14, Manolis Stamatogiannakis
wrote:
> Allows shifting of subtitle display times to align them with the video.
> This avoids having to rewrite the subtitle file in order to display
> subtitles correctly when input is seeked (-ss).
> Also handy for minor subtitle timing corrections without rewriting the
> subtitles file.
>
> Signed-off-by: Manolis Stamatogiannakis
> ---
> doc/filters.texi | 8
> libavfilter/vf_subtitles.c | 29 +++--
> 2 files changed, 35 insertions(+), 2 deletions(-)
>
> diff --git a/doc/filters.texi b/doc/filters.texi
> index d19fd346ae..94323495f0 100644
> --- a/doc/filters.texi
> +++ b/doc/filters.texi
> @@ -17851,6 +17851,9 @@ The filter accepts the following options:
> @item filename, f
> Set the filename of the subtitle file to read. It must be specified.
>
> +@item shift
> +Shift subtitles timings by the specified amount.
> +
> @item original_size
> Specify the size of the original video, the video for which the ASS file
> was composed. For the syntax of this option, check the
> @@ -17907,6 +17910,11 @@ To make the subtitles stream from @file{sub.srt}
> appear in 80% transparent blue
> subtitles=sub.srt:force_style='FontName=DejaVu
> Serif,PrimaryColour='
> @end example
>
> +To re-sync subtitles after seeking the input e.g. with @code{-ss 20:20},
> use:
> +@example
> +subtitles=filename=sub.srt:shift=-20\:20
> +@end example
> +
> @section super2xsai
>
> Scale the input by 2x and smooth using the Super2xSaI (Scale and
> diff --git a/libavfilter/vf_subtitles.c b/libavfilter/vf_subtitles.c
> index a3b4029af4..47a38b55b1 100644
> --- a/libavfilter/vf_subtitles.c
> +++ b/libavfilter/vf_subtitles.c
> @@ -52,6 +52,8 @@ typedef struct AssContext {
> char *filename;
> char *fontsdir;
> char *charenc;
> +int64_t shift;
> +char *shift_opt;
> char *force_style;
> int stream_index;
> int alpha;
> @@ -68,6 +70,7 @@ typedef struct AssContext {
> #define COMMON_OPTIONS \
> {"filename", "set the filename of file to read",
> OFFSET(filename), AV_OPT_TYPE_STRING, {.str = NULL}, 0, 0,
> FLAGS }, \
> {"f", "set the filename of file to read",
> OFFSET(filename), AV_OPT_TYPE_STRING, {.str = NULL}, 0, 0,
> FLAGS }, \
> +{"shift", "shift the timing of the subtitles",
> OFFSET(shift_opt), AV_OPT_TYPE_STRING, {.str = NULL}, 0, 0,
> FLAGS }, \
> {"original_size", "set the size of the original video (used to scale
> fonts)", OFFSET(original_w), AV_OPT_TYPE_IMAGE_SIZE, {.str = NULL}, 0, 0,
> FLAGS }, \
> {"fontsdir", "set the directory containing the fonts to read",
> OFFSET(fontsdir), AV_OPT_TYPE_STRING, {.str = NULL}, 0, 0,
> FLAGS }, \
> {"alpha", "enable processing of alpha channel",
> OFFSET(alpha), AV_OPT_TYPE_BOOL, {.i64 = 0 },
> 0,1, FLAGS }, \
> @@ -103,6 +106,16 @@ static av_cold int init(AVFilterContext *ctx)
> return AVERROR(EINVAL);
> }
>
> +if (ass->shift_opt) {
> +if (av_parse_time(>shift, ass->shift_opt, 1) < 0) {
> +av_log(ctx, AV_LOG_ERROR, "Invalid subtitles shift: %s\n",
> + ass->shift_opt);
> +return AVERROR(EINVAL);
> +}
> +ass->shift = av_rescale_q(ass->shift, AV_TIME_BASE_Q,
> av_make_q(1, 1000));
> +av_log(ctx, AV_LOG_DEBUG, "Shifting subtitles by %0.3fsec.\n",
> ass->shift/1000.0);
> +}
> +
> ass->library = ass_library_init();
> if (!ass->library) {
> av_log(ctx, AV_LOG_ERROR, "Could not initialize libass.\n");
> @@ -297,7 +310,7 @@ AVFILTER_DEFINE_CLASS(subtitles);
>
> static av_cold int init_subtitles(AVFilterContext *ctx)
> {
> -int j, ret, sid;
> +int j, ret, sid, nskip;
> int k = 0;
> AVDictionary *codec_opts = NULL;
> AVFormatContext *fmt = NULL;
> @@ -448,6 +461,7 @@ static av_cold int init_subtitles(AVFilterContext *ctx)
> av_init_packet();
> pkt.data = NULL;
> pkt.size = 0;
> +nskip = 0;
> while (av_read_frame(fmt, ) >= 0) {
> int i, got_subtitle;
> AVSubtitle sub = {0};
> @@ -458,8 +472,17 @@ static av_cold int init_subtitles(AVFilterContext
> *ctx)
> av_log(ctx, AV_LOG_WARNING, "Error decoding: %s
> (ignored)\n",
>
[FFmpeg-devel] [PATCH] avfilter/vf_subtitles: add shift option
Allows shifting of subtitle display times to align them with the video.
This avoids having to rewrite the subtitle file in order to display
subtitles correctly when input is seeked (-ss).
Also handy for minor subtitle timing corrections without rewriting the
subtitles file.
Signed-off-by: Manolis Stamatogiannakis
---
doc/filters.texi | 8
libavfilter/vf_subtitles.c | 29 +++--
2 files changed, 35 insertions(+), 2 deletions(-)
diff --git a/doc/filters.texi b/doc/filters.texi
index d19fd346ae..94323495f0 100644
--- a/doc/filters.texi
+++ b/doc/filters.texi
@@ -17851,6 +17851,9 @@ The filter accepts the following options:
@item filename, f
Set the filename of the subtitle file to read. It must be specified.
+@item shift
+Shift subtitles timings by the specified amount.
+
@item original_size
Specify the size of the original video, the video for which the ASS file
was composed. For the syntax of this option, check the
@@ -17907,6 +17910,11 @@ To make the subtitles stream from @file{sub.srt}
appear in 80% transparent blue
subtitles=sub.srt:force_style='FontName=DejaVu Serif,PrimaryColour='
@end example
+To re-sync subtitles after seeking the input e.g. with @code{-ss 20:20}, use:
+@example
+subtitles=filename=sub.srt:shift=-20\:20
+@end example
+
@section super2xsai
Scale the input by 2x and smooth using the Super2xSaI (Scale and
diff --git a/libavfilter/vf_subtitles.c b/libavfilter/vf_subtitles.c
index a3b4029af4..47a38b55b1 100644
--- a/libavfilter/vf_subtitles.c
+++ b/libavfilter/vf_subtitles.c
@@ -52,6 +52,8 @@ typedef struct AssContext {
char *filename;
char *fontsdir;
char *charenc;
+int64_t shift;
+char *shift_opt;
char *force_style;
int stream_index;
int alpha;
@@ -68,6 +70,7 @@ typedef struct AssContext {
#define COMMON_OPTIONS \
{"filename", "set the filename of file to read",
OFFSET(filename), AV_OPT_TYPE_STRING, {.str = NULL}, 0, 0, FLAGS }, \
{"f", "set the filename of file to read",
OFFSET(filename), AV_OPT_TYPE_STRING, {.str = NULL}, 0, 0, FLAGS }, \
+{"shift", "shift the timing of the subtitles",
OFFSET(shift_opt), AV_OPT_TYPE_STRING, {.str = NULL}, 0, 0, FLAGS }, \
{"original_size", "set the size of the original video (used to scale
fonts)", OFFSET(original_w), AV_OPT_TYPE_IMAGE_SIZE, {.str = NULL}, 0, 0,
FLAGS }, \
{"fontsdir", "set the directory containing the fonts to read",
OFFSET(fontsdir), AV_OPT_TYPE_STRING, {.str = NULL}, 0, 0, FLAGS }, \
{"alpha", "enable processing of alpha channel",
OFFSET(alpha), AV_OPT_TYPE_BOOL, {.i64 = 0 }, 0,
1, FLAGS }, \
@@ -103,6 +106,16 @@ static av_cold int init(AVFilterContext *ctx)
return AVERROR(EINVAL);
}
+if (ass->shift_opt) {
+if (av_parse_time(>shift, ass->shift_opt, 1) < 0) {
+av_log(ctx, AV_LOG_ERROR, "Invalid subtitles shift: %s\n",
+ ass->shift_opt);
+return AVERROR(EINVAL);
+}
+ass->shift = av_rescale_q(ass->shift, AV_TIME_BASE_Q, av_make_q(1,
1000));
+av_log(ctx, AV_LOG_DEBUG, "Shifting subtitles by %0.3fsec.\n",
ass->shift/1000.0);
+}
+
ass->library = ass_library_init();
if (!ass->library) {
av_log(ctx, AV_LOG_ERROR, "Could not initialize libass.\n");
@@ -297,7 +310,7 @@ AVFILTER_DEFINE_CLASS(subtitles);
static av_cold int init_subtitles(AVFilterContext *ctx)
{
-int j, ret, sid;
+int j, ret, sid, nskip;
int k = 0;
AVDictionary *codec_opts = NULL;
AVFormatContext *fmt = NULL;
@@ -448,6 +461,7 @@ static av_cold int init_subtitles(AVFilterContext *ctx)
av_init_packet();
pkt.data = NULL;
pkt.size = 0;
+nskip = 0;
while (av_read_frame(fmt, ) >= 0) {
int i, got_subtitle;
AVSubtitle sub = {0};
@@ -458,8 +472,17 @@ static av_cold int init_subtitles(AVFilterContext *ctx)
av_log(ctx, AV_LOG_WARNING, "Error decoding: %s (ignored)\n",
av_err2str(ret));
} else if (got_subtitle) {
-const int64_t start_time = av_rescale_q(sub.pts,
AV_TIME_BASE_Q, av_make_q(1, 1000));
+const int64_t start_time = av_rescale_q(sub.pts,
AV_TIME_BASE_Q, av_make_q(1, 1000)) + ass->shift;
const int64_t duration = sub.end_display_time;
+
+if (start_time + duration < 0) {
+nskip++;
+goto pkt_end;
+} else if (nskip > 0) {
+av_log(ctx, AV_LOG_INFO, "Skipped %
Re: [FFmpeg-devel] [PATCH] avfilter/vf_subtitles: add shift options
Good catch. I've fixed the text and resubmitted.
Thanks.
Manolis
On Fri, 8 May 2020 at 12:33, Bodecs Bela wrote:
> see may comment in text
>
> 2020.05.03. 20:08 keltezéssel, Manolis Stamatogiannakis írta:
> > Allows shifting of subtitle display times to align them with the video.
> > This avoids having to rewrite the subtitle file in order to display
> > subtitles correctly when input is seeked (-ss).
> > Also handy for minor subtitle timing corrections without rewriting the
> > subtitles file.
> >
> > Signed-off-by: Manolis Stamatogiannakis
> > ---
> > doc/filters.texi | 8
> > libavfilter/vf_subtitles.c | 29 +++--
> > 2 files changed, 35 insertions(+), 2 deletions(-)
> >
> > diff --git a/doc/filters.texi b/doc/filters.texi
> > index d19fd346ae..94323495f0 100644
> > --- a/doc/filters.texi
> > +++ b/doc/filters.texi
> > @@ -17851,6 +17851,9 @@ The filter accepts the following options:
> > @item filename, f
> > Set the filename of the subtitle file to read. It must be specified.
> >
> > +@item shift
> > +Shift subtitles timings by the specified amount.
> > +
> > @item original_size
> > Specify the size of the original video, the video for which the ASS
> file
> > was composed. For the syntax of this option, check the
> > @@ -17907,6 +17910,11 @@ To make the subtitles stream from
> @file{sub.srt} appear in 80% transparent blue
> > subtitles=sub.srt:force_style='FontName=DejaVu
> Serif,PrimaryColour='
> > @end example
> >
> > +To re-sync subtitles after seeking the input e.g. with @code{-ss
> 20:20}, use:
> > +@example
> > +subtitles=filename=sub.srt:shift=-20\:20
> > +@end example
> > +
> > @section super2xsai
> >
> > Scale the input by 2x and smooth using the Super2xSaI (Scale and
> > diff --git a/libavfilter/vf_subtitles.c b/libavfilter/vf_subtitles.c
> > index a3b4029af4..74a902941a 100644
> > --- a/libavfilter/vf_subtitles.c
> > +++ b/libavfilter/vf_subtitles.c
> > @@ -52,6 +52,8 @@ typedef struct AssContext {
> > char *filename;
> > char *fontsdir;
> > char *charenc;
> > +int64_t shift;
> > +char *shift_opt;
> > char *force_style;
> > int stream_index;
> > int alpha;
> > @@ -68,6 +70,7 @@ typedef struct AssContext {
> > #define COMMON_OPTIONS \
> > {"filename", "set the filename of file to read",
> OFFSET(filename), AV_OPT_TYPE_STRING, {.str = NULL}, 0,
> 0, FLAGS }, \
> > {"f", "set the filename of file to read",
> OFFSET(filename), AV_OPT_TYPE_STRING, {.str = NULL}, 0,
> 0, FLAGS }, \
> > +{"shift", "set the tilename of file to read",
>OFFSET(shift_opt), AV_OPT_TYPE_STRING, {.str = NULL}, 0,
> 0, FLAGS }, \
> desciption is wrong:
> > {"original_size", "set the size of the original video (used to
> scale fonts)", OFFSET(original_w), AV_OPT_TYPE_IMAGE_SIZE, {.str = NULL},
> 0, 0, FLAGS }, \
> > {"fontsdir", "set the directory containing the fonts to
> read", OFFSET(fontsdir), AV_OPT_TYPE_STRING, {.str =
> NULL}, 0, 0, FLAGS }, \
> > {"alpha", "enable processing of alpha channel",
> OFFSET(alpha), AV_OPT_TYPE_BOOL, {.i64 = 0 },
> 0,1, FLAGS }, \
> > @@ -103,6 +106,16 @@ static av_cold int init(AVFilterContext *ctx)
> > return AVERROR(EINVAL);
> > }
> >
> > +if (ass->shift_opt) {
> > +if (av_parse_time(>shift, ass->shift_opt, 1) < 0) {
> > +av_log(ctx, AV_LOG_ERROR, "Invalid subtitles shift: %s\n",
> > + ass->shift_opt);
> > +return AVERROR(EINVAL);
> > +}
> > +ass->shift = av_rescale_q(ass->shift, AV_TIME_BASE_Q,
> av_make_q(1, 1000));
> > +av_log(ctx, AV_LOG_DEBUG, "Shifting subtitles by %0.3fsec.\n",
> ass->shift/1000.0);
> > +}
> > +
> > ass->library = ass_library_init();
> > if (!ass->library) {
> > av_log(ctx, AV_LOG_ERROR, "Could not initialize libass.\n");
> > @@ -297,7 +310,7 @@ AVFILTER_DEFINE_CLASS(subtitles);
> >
> > static av_cold int init_subtitles(AVFilterContext *ctx)
> > {
> > -int j, ret, sid;
> > +int j, ret, sid,
Re: [FFmpeg-devel] [PATCH] drawtext: Allow textfile path to be expanded per frame
Hi David, Not a full review, but a minor point that popped in mind. It is common to pad serially numbered files with 0. E.g. if you have <1000 files, the 9th file will be named file009.txt. Is this case handled by the expansion? Or is it assumed that the text file numbers are not zero-padded? If it the latter is the case, it would be good to make that explicit in the documentation. Of course, accounting for padding would be better, but this may not be straightforward without adding more code. Regards, Manolis On Tue, 19 May 2020 at 06:10, David Andreoletti wrote: > Hi ffmpeg maintainers / community, > New contributor here. The patch [0] for the drawtext filter was submitted > some > time ago now but has been reviewed yet. It seems there is no official > maintainer > for this filter (as per the MAINTAINERS file) > What should be done to have it reviewed ? > > [0] > > https://patchwork.ffmpeg.org/project/ffmpeg/patch/20200511143159.19390-1-da...@andreoletti.net/ > > Regards, > David Andreoletti > > > ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] subtitles filter and -ss
Thanks for the tip Nicolas. Well, -copyts works fine when you're re-encoding at full speed. But when used in combination with -re, as shown below, it prevents the input from being "fast-seeked" to the desired position. So, it's kind of useless. ffmpeg -re -copyts -ss 20:10.00 -i input.mp4 -vf subtitles=subs.srt \ -c:v libx264 crf 25 -c:a aac -ab 160k \ -strict experimental \ -f flv $RTMP_URL I'm not intimate enough with the code to tell if that's a bug or an inherent limitation of -copyts. OTOH, the shift option added to the subtitles filter with the patch does not prevent fast-seeking. And you also have the added benefit of adjusting subtitles delay without having to rewrite them. Regards, Manolis On Mon, 4 May 2020 at 11:38, Nicolas George wrote: > Manolis Stamatogiannakis (12020-05-03): > > I've noticed what appears to be a bug/missing feature in the subtitles > > filter: when "-ss" is used for the input, it is not applied to the > > subtitles stream. E.g., for the following command line, the video > playback > > will start on 20:10, but the subtitles will start from 00:00. > > You can use -copyts to keep the timestamps of the video matching with > the subtitles. > > Regards, > > -- > Nicolas George > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe". ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH v1 1/3] doc/developer.texi: Improvements in "Submitting patches" section.
The section has been expanded to outline how to manage patch revisions.
---
doc/developer.texi | 37 ++---
1 file changed, 26 insertions(+), 11 deletions(-)
diff --git a/doc/developer.texi b/doc/developer.texi
index b33cab0fc7..dec27cb509 100644
--- a/doc/developer.texi
+++ b/doc/developer.texi
@@ -491,18 +491,25 @@ as base64-encoded attachments, so your patch is not
trashed during
transmission. Also ensure the correct mime type is used
(text/x-diff or text/x-patch or at least text/plain) and that only one
patch is inline or attached per mail.
-You can check @url{https://patchwork.ffmpeg.org}, if your patch does not show
up, its mime type
-likely was wrong.
+You can check the most recently received patches on
+@url{https://patchwork.ffmpeg.org, patchwork}.
+If your patch does not show up, its mime type likely was wrong.
-Your patch will be reviewed on the mailing list. You will likely be asked
+Your patch will be reviewed on the mailing list. Give us a few days to react.
+But if some time passes without reaction, send a reminder by email.
+Your patch should eventually be dealt with. However, you will likely be asked
to make some changes and are expected to send in an improved version that
incorporates the requests from the review. This process may go through
several iterations. Once your patch is deemed good enough, some developer
will pick it up and commit it to the official FFmpeg tree.
-Give us a few days to react. But if some time passes without reaction,
-send a reminder by email. Your patch should eventually be dealt with.
-
+When preparing an updated version of you patch, make sure you increment
+its @emph{roll-counter}. This is achieved by adding a @code{-v } argument
+to @code{git format-patch}/@code{git send-email} commands.
+Additionally, it is recommended to register for a
+@uref{https://patchwork.ffmpeg.org, patchwork account}.
+This will allow you to mark previous version of your patches as "Superseded",
+and reduce the chance of someone spending time to review a stale patch.
@chapter New codecs or formats checklist
@@ -563,7 +570,19 @@ Did you make sure it compiles standalone, i.e. with
Does @code{make fate} pass with the patch applied?
@item
-Was the patch generated with git format-patch or send-email?
+Was the patch generated with @code{git format-patch} or @code{git send-email}?
+
+@item
+If you are submitting a revised patch, did you increment the roll-counter
+with @code{-v }?
+
+@item
+If you are submitting a revised patch, did you marked previous versions of
+the patch as "Superseded" on @uref{https://patchwork.ffmpeg.org/, patchwork}?
+
+@item
+Are you subscribed to ffmpeg-devel?
+(the list is subscribers only due to spam)
@item
Did you sign-off your patch? (@code{git commit -s})
@@ -576,10 +595,6 @@ Did you provide a clear git commit log message?
@item
Is the patch against latest FFmpeg git master branch?
-@item
-Are you subscribed to ffmpeg-devel?
-(the list is subscribers only due to spam)
-
@item
Have you checked that the changes are minimal, so that the same cannot be
achieved with a smaller patch and/or simpler final code?
--
2.17.1
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] patch submission questions
Thanks for the tips Andriy. To make it easier for future new contributors I took the time to update developers.texi with the information I got from this thread, and revamp the chapter about submitting patches. Feedback is most welcome. Best regards, Manolis On Sun, 5 Jul 2020 at 20:56, Andriy Gelman wrote: > On Sun, 05. Jul 20:34, Manolis Stamatogiannakis wrote: > > On Sun, 5 Jul 2020 at 15:48, Hongcheng Zhong > > wrote: > > > > > > > > You can use `git format-patch -v -n` to get patches like [PATCH > > > v2 1/20]. See git-format-patch documentation for more details. > > > > > > > > That didn't quite work. > > > > I used "git format-patch -s -n -v3 --to ffmpeg-devel@ffmpeg.org > > HEAD~2..HEAD" and "git send-email -2". > > > > But the -v argument was dropped and the end results on patchwork were > again: > > [FFmpeg-devel,2/2] avfilter/vf_subtitles: Added shift option for > > subtitles/ass filters. > > [FFmpeg-devel,1/2] avfilter/vf_subtitles: Reorganized subtitles filter > > options. > > > > git send-email -v2 HEAD~ # works for me > > git shows are draft of the email before sending. Check that the subject > line > contains the version number (or you can send the patch to yourself) > > For patchwork, you can create an account and set your old patches as > 'Superseded'. > I also periodically run a script that sets Superseded and Accepted labels. > > -- > Andriy > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe". ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH v1 3/3] doc/developer.texi: Swapped patch checklist and new codec/format checklist.
Adding a new codec/format should be more rare, so it makes sense
to come after the detailed patch submission checklist.
Signed-off-by: Manolis Stamatogiannakis
---
doc/developer.texi | 105 ++---
1 file changed, 52 insertions(+), 53 deletions(-)
diff --git a/doc/developer.texi b/doc/developer.texi
index 6d4f6afcf9..cecb10fed1 100644
--- a/doc/developer.texi
+++ b/doc/developer.texi
@@ -529,59 +529,6 @@ Additionally, it is recommended to register for a
This will allow you to mark previous version of your patches as "Superseded",
and reduce the chance of someone spending time to review a stale patch.
-@anchor{new codec format checklist}
-@section New codecs or formats checklist
-
-@enumerate
-@item
-Did you use av_cold for codec initialization and close functions?
-
-@item
-Did you add a long_name under NULL_IF_CONFIG_SMALL to the AVCodec or
-AVInputFormat/AVOutputFormat struct?
-
-@item
-Did you bump the minor version number (and reset the micro version
-number) in @file{libavcodec/version.h} or @file{libavformat/version.h}?
-
-@item
-Did you register it in @file{allcodecs.c} or @file{allformats.c}?
-
-@item
-Did you add the AVCodecID to @file{avcodec.h}?
-When adding new codec IDs, also add an entry to the codec descriptor
-list in @file{libavcodec/codec_desc.c}.
-
-@item
-If it has a FourCC, did you add it to @file{libavformat/riff.c},
-even if it is only a decoder?
-
-@item
-Did you add a rule to compile the appropriate files in the Makefile?
-Remember to do this even if you're just adding a format to a file that is
-already being compiled by some other rule, like a raw demuxer.
-
-@item
-Did you add an entry to the table of supported formats or codecs in
-@file{doc/general.texi}?
-
-@item
-Did you add an entry in the Changelog?
-
-@item
-If it depends on a parser or a library, did you add that dependency in
-configure?
-
-@item
-Did you @code{git add} the appropriate files before committing?
-
-@item
-Did you make sure it compiles standalone, i.e. with
-@code{configure --disable-everything --enable-decoder=foo}
-(or @code{--enable-demuxer} or whatever your component is)?
-@end enumerate
-
-
@anchor{patch submission checklist}
@section Patch submission checklist
@@ -708,6 +655,58 @@ Test your code with valgrind and or Address Sanitizer to
ensure it's free
of leaks, out of array accesses, etc.
@end enumerate
+@anchor{new codec format checklist}
+@section New codecs or formats checklist
+
+@enumerate
+@item
+Did you use av_cold for codec initialization and close functions?
+
+@item
+Did you add a long_name under NULL_IF_CONFIG_SMALL to the AVCodec or
+AVInputFormat/AVOutputFormat struct?
+
+@item
+Did you bump the minor version number (and reset the micro version
+number) in @file{libavcodec/version.h} or @file{libavformat/version.h}?
+
+@item
+Did you register it in @file{allcodecs.c} or @file{allformats.c}?
+
+@item
+Did you add the AVCodecID to @file{avcodec.h}?
+When adding new codec IDs, also add an entry to the codec descriptor
+list in @file{libavcodec/codec_desc.c}.
+
+@item
+If it has a FourCC, did you add it to @file{libavformat/riff.c},
+even if it is only a decoder?
+
+@item
+Did you add a rule to compile the appropriate files in the Makefile?
+Remember to do this even if you're just adding a format to a file that is
+already being compiled by some other rule, like a raw demuxer.
+
+@item
+Did you add an entry to the table of supported formats or codecs in
+@file{doc/general.texi}?
+
+@item
+Did you add an entry in the Changelog?
+
+@item
+If it depends on a parser or a library, did you add that dependency in
+configure?
+
+@item
+Did you @code{git add} the appropriate files before committing?
+
+@item
+Did you make sure it compiles standalone, i.e. with
+@code{configure --disable-everything --enable-decoder=foo}
+(or @code{--enable-demuxer} or whatever your component is)?
+@end enumerate
+
@chapter Patch review process
All patches posted to ffmpeg-devel will be reviewed, unless they contain a
--
2.17.1
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH v1 2/3] doc/developer.texi: Restructured "Submitting patches" section.
- Main text split to two sections.
- Detailed checklist for new codecs or formats demoted to section.
- Detailed checklist for patch submission demoted to section.
Signed-off-by: Manolis Stamatogiannakis
---
doc/developer.texi | 64 +++---
1 file changed, 44 insertions(+), 20 deletions(-)
diff --git a/doc/developer.texi b/doc/developer.texi
index dec27cb509..6d4f6afcf9 100644
--- a/doc/developer.texi
+++ b/doc/developer.texi
@@ -457,31 +457,49 @@ Finally, keep in mind the immortal words of Bill and Ted,
@anchor{Submitting patches}
@chapter Submitting patches
-First, read the @ref{Coding Rules} above if you did not yet, in particular
+@anchor{patch guidelines}
+@section Guidelines for preparing a patch
+
+The @strong{absolute minimum} you have to do before submitting a patch are the
+following:
+
+@enumerate
+@item Carefully read the @ref{Coding Rules} above if you did not yet, in
particular
the rules regarding patch submission.
-When you submit your patch, please use @code{git format-patch} or
-@code{git send-email}. We cannot read other diffs :-).
+@item Make sure your commit messages accurately describe the changes made
+(e.g. 'replaces lrint by lrintf') and why these changes are made (e.g.
+'*BSD isn't C99 compliant and has no lrint()').
-Also please do not submit a patch which contains several unrelated changes.
-Split it into separate, self-contained pieces. This does not mean splitting
-file by file. Instead, make the patch as small as possible while still
-keeping it as a logical unit that contains an individual change, even
-if it spans multiple files. This makes reviewing your patches much easier
-for us and greatly increases your chances of getting your patch applied.
+@item Make sure you use @code{git format-patch} or @code{git send-email} to
prepare
+your patch. We cannot read other diffs :-).
+
+@item Run the @ref{Regression tests, regression tests} before submitting a
patch
+in order to verify it does not cause unexpected problems.
-Use the patcheck tool of FFmpeg to check your patch.
-The tool is located in the tools directory.
+@item If you send your patches with an external email client
+(i.e. not @code{git send-email}), make sure to send each patch as a separate
+email. Do not attach several patches to the same email!
-Run the @ref{Regression tests} before submitting a patch in order to verify
-it does not cause unexpected problems.
+@item Do not submit a patch which contains several unrelated changes.
+@end enumerate
+
+Additionally, it is also important that the commits comprising a patch
+are logically self-contained. I.e. each commit should be as small as
+possible while still containing a meaningful individual change.
+Commits spanning multiple files are perfectly fine, as long as the
+commit can be seen as a single logical unit.
-It also helps quite a bit if you tell us what the patch does (for example
-'replaces lrint by lrintf'), and why (for example '*BSD isn't C99 compliant
-and has no lrint()')
+Following these guidelines makes reviewing your patches much easier
+for us and greatly increases your chances of getting your patch applied.
+To further reduce the chance that you will need to revise your patch,
+it is also recommended to go through the detailed
+@ref{patch submission checklist, patch} and
+@ref{new codec format checklist, new codec or format}
+checklists.
-Also please if you send several patches, send each patch as a separate mail,
-do not attach several unrelated patches to the same mail.
+@anchor{patch submission process}
+@section Patch submission and revision process
Patches should be posted to the
@uref{https://lists.ffmpeg.org/mailman/listinfo/ffmpeg-devel, ffmpeg-devel}
@@ -511,7 +529,8 @@ Additionally, it is recommended to register for a
This will allow you to mark previous version of your patches as "Superseded",
and reduce the chance of someone spending time to review a stale patch.
-@chapter New codecs or formats checklist
+@anchor{new codec format checklist}
+@section New codecs or formats checklist
@enumerate
@item
@@ -563,7 +582,8 @@ Did you make sure it compiles standalone, i.e. with
@end enumerate
-@chapter Patch submission checklist
+@anchor{patch submission checklist}
+@section Patch submission checklist
@enumerate
@item
@@ -592,6 +612,10 @@ of @dfn{sign-off}.
@item
Did you provide a clear git commit log message?
+@item
+Did you use the @code{patcheck} tool of FFmpeg to check your patch
+for common issues? E.g. @code{tools/patcheck *.patch}.
+
@item
Is the patch against latest FFmpeg git master branch?
--
2.17.1
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] Project orientation
Thanks Gerion. This sounds more or less like what I had in mind (but I was trying to keep it short). – The present thread is a non-technical discussion, and branching indeed helps. The mailing list is perfect for this kind of discussions. – Patches with narrow scope are focused, technical discussion, which is unlikely to branch. They can be dispatched and discussed directly as a PR, either via the web interface or email. This should keep this list more focused. – Patches that involve deeper changes can be discussed before materializing in a PR, in whatever way core developers prefer. To be clear, by no means I am in favour of deprecating the mailing list. PRs and Issues are not a panacea. E.g. asking questions by opening issues on GH/GL is something I would frown upon (and I have seen happening elsewhere). Best regards, Manolis On Thu, 9 Jul 2020 at 12:27, Gerion Entrup wrote: > Hi, > > Am Donnerstag, 9. Juli 2020, 02:56:28 CEST schrieb Manolis > Stamatogiannakis: > > On Tue, 7 Jul 2020 at 16:27, Nicolas George wrote: > > > > Is tree threading that important? A PR is essentially a single > thread of > > > > discussion. > > > > > > It is a single thread of discussion until the discussion becomes > complex > > > and has branches. > > > > > > > This doesn't sound like the common case. > > But it should be straightforward to get some statistics on that from the > > list archives when a transition is officially discussed. > > This whole current discussion here would be a lot more confusing without > branches. > > Maybe you like the Gentoo approach: Do the majority of changes in pull > requests, in Gentoo this are changes of packages, here it would be > changes on codecs/demuxers/filters/etc. And then do all changes of the > framework itself and discussions via mailinglist. In Gentoo this are > changes of eclasses (i.e. code libraries for packages). For FFmpeg it > would be the introduction of new API, API changes, decoupling or merging > of libraries etc. > > The first is likely to produce no to little (conflicting) discussion, > has a huge benefit from CI, is the part where most of the newcomers begin > development and includes a lot of easy tasks. Here by far the most > developments happens. > > The latter is likely to produce huge discussions, needs deep knowledge > of the libraries, so it is not likely done by newcomers and has little > gain from CI since it adds e.g. something completely new. Here happens > not so much development in terms of frequency or code lines but the > impact is much higher. > > Just a suggestion, since I follow both projects... > > Regards, > Gerion > > > > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe". ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] Project orientation
On Tue, 7 Jul 2020 at 13:31, Nicolas George wrote: > Manolis Stamatogiannakis (12020-07-07): > > We don't live in a world of innocence. Enabling "less secure" > applications > > is trouble waiting to happen. > > Please don't believe that "less secure" applications are actually less > secure. The only thing they are less secure for is advertisers' business > model. > > I believe I have adequately explained that "less secure" needs to be enabled, not to make your Google account more secure, but to contain the damage from a potential compromise. Yes, "less secure" sounds a lot like marketing slang. But Gmail is not used exclusively by CompSci majors :) Best regards, Manolis ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] Project orientation
On Tue, 7 Jul 2020 at 12:46, Nicolas George wrote: > > * GMail's warnings about "less secure" applications are scare tactics to > get you to exclusively use their products, because they cannot feed > you advertisement when you use a real mail client with their IMAP and > SMTP servers. > > I *strongly* disagree with the scare tactics part. One's gmail account may be tied to several other online accounts. Having "less secure" applications enabled means that if your gmail password is compromised, people can easily own a few dozens of other accounts you own. And you won't even know, because gmail won't alert you about the intrusion. We don't live in a world of innocence. Enabling "less secure" applications is trouble waiting to happen. Best regards, Manolis ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] Project orientation
On Tue, 7 Jul 2020 at 14:59, Nicolas George wrote: > > Is there a way of interacting with the discussions in GitHub issues > outside their web interface? > > If there is, I never found it. If there is not, then GitHub's issue > system is just not usable for serious work. > Any activity on issues/PRs I watch on GitHub is also sent to me with email. If I reply to the email, my response will appear online in the issue/PR page. I've been doing that for years. When was the last time you checked? Now, if you also want to review the code of a PRs from mutt, that's another discussion. > (Also, who designs discussion systems without tree threading?!?) > Is tree threading that important? A PR is essentially a single thread of discussion. If discussion on the PR carries on, you will anyway start stripping the old context and respond to the bits of the last email. So tree threading doesn't do much. I would first ask why keep using a mailing list to post patches and not use PRs. For a 3 commit patch, and 3 people sending feedback a few days apart, you have a minimum of 15 emails for everyone on the list. Is that efficient? Best regards, Manolis ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] Project orientation
On Tue, 7 Jul 2020 at 14:38, Nicolas George wrote: > Manolis Stamatogiannakis (12020-07-07): > > I believe I have adequately explained that "less secure" needs to be > > enabled, not to make your Google account more secure, but to contain > > the damage from a potential compromise. > > > > Yes, "less secure" sounds a lot like marketing slang. But Gmail is not > used > > exclusively by CompSci majors :) > > I will state it another way. > > Anybody should be able to use the software of their choice to read their > e-mail. > > If a compromise around these software have a consequence beyond mail and > what is directly related to mail, then the fault is not the software's, > the fault is to whoever tied mail and other things together. > > Google has a totalitarian policy, as in they want to totally control > your web experience (the fact that the same word is used to qualify > dictatorships is not a coincidence): they make every effort so that if > you dip a toe in them, you find yourself submerged before you know it. > > It is Google's choice, and Google's responsibility, and your choice and > your responsibility if you decide to use Google totally. > > This choice should not direct the workings of a Libre Software projects > that is not under the control of Google. The choice should even less > direct the choices of other people working on the project. > > As was pointed out earlier, there are solutions. Even if Google hides > it, there are ways of accessing their walled gardens with other > applications. Even if there are not, you can decide to isolate your > accounts. (If would probably be a good idea irregardless of FFmpeg, > anyway.) > > So learn to use them, and do not demand other people to change their > (good) habits so that you can keep your (bad) habits. > I understand your concerns, but Google has been pretty open with all its productivity SaaS offerings. So, while I would be against mandating the use of any Google products, I can't see catering for Gmail users as a valid existential threat to any open/libre project. And some quick grepping reveals that 45% of the emails in the list for 2019-2020 were from 236 unique Gmail addresses. That's an awful lot of people with bad habits to ignore. > I would offer my help in doing so, but as I explained, I do not use > Google, and therefore have no help to offer to better use it. But if you > want my help in learning powerful command-line tools, you can have it. > (And it would probably be a good investment of your time.) > > I'm a command line veteran myself, with the same weapons of choice as you (vim/zsh), but I still struggle with the ffmpeg workflow. I'd surely be interested for any command line tools that would make it easier. If you have the time to add a section to the documentation, I'm sure it would be helpful for many people. And there's also the "tools" directory for sharing any tools/scripts you created for ffmpeg development. Best regards, Manolis ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v1 2/3] doc/developer.texi: Restructured "Submitting patches" section.
On Tue, 7 Jul 2020 at 16:07, Nicolas George wrote:
> Manolis Stamatogiannakis (12020-07-05):
>
> > +@item If you send your patches with an external email client
> > +(i.e. not @code{git send-email}), make sure to send each patch as a
> separate
> > +email. Do not attach several patches to the same email!
>
> This is a new rule, it did not exist before, and I see little value in
> it except making Patchwork happy.
>
The current documentation mentions:
L1: Also please do not submit a patch which contains several unrelated
changes.
...
L2: Also please if you send several patches, send each patch as a separate
mail, do not attach several unrelated patches to the same mail.
...
L3: Use git send-email when possible since it will properly send patches
without requiring extra care.
The rule in the list is a summary of these three lines.
I may have interpreted them wrong, as there's a slight overlap (L1-L2: they
look the same) and a slight conflict (L2-L3: git send-email sends one email
per commit, not per patch).
Since you send your patches with an external email client, can you come up
with a better/more accurate phrasing based on your workflow?
> >
> > -Run the @ref{Regression tests} before submitting a patch in order to
> verify
> > -it does not cause unexpected problems.
> > +@item Do not submit a patch which contains several unrelated changes.
> > +@end enumerate
> > +
>
> > +Additionally, it is also important that the commits comprising a patch
> > +are logically self-contained. I.e. each commit should be as small as
>
> Uh? Are you making a distinction between commits and patches? So, can we
> have a single patch with several commits in one mail?
>
> Or maybe the accurate wording is just not consistent.
>
I believe that much of the wording in developer.html stems from the svn
days, so it reads a bit funny if you've been using git exclusively for 5+
years.
Here are the number of lines committed per year in developer.texi:
2014: 3
2018: 8
2012: 21
2015: 29
2017: 50
2016: 86
2011: 105
2020: 124
2009: 127
2013: 351
So let's stick with "patches" and forget about commits for now. Does this
sound better?
"Additionally, it is also important that each patch is logically
self-contained.
I.e. each patch should be as small as possible while still containing a
meaningful
individual change.
Patches spanning multiple files are perfectly fine, as long as they can be
seen
as a single logical unit."
Thanks for the review!
Best regards,
Manolis
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH] avfilter/vf_subtitles: add shift option
Allows shifting of subtitle display times to align them with the video.
This avoids having to rewrite the subtitle file in order to display
subtitles correctly when input is seeked (-ss).
Also handy for minor subtitle timing corrections without rewriting the
subtitles file.
Signed-off-by: Manolis Stamatogiannakis
---
doc/filters.texi | 8
libavfilter/vf_subtitles.c | 23 +--
2 files changed, 29 insertions(+), 2 deletions(-)
diff --git a/doc/filters.texi b/doc/filters.texi
index ad2448acb2..c962ac55b0 100644
--- a/doc/filters.texi
+++ b/doc/filters.texi
@@ -17935,6 +17935,9 @@ The filter accepts the following options:
@item filename, f
Set the filename of the subtitle file to read. It must be specified.
+@item shift
+Shift subtitles timings by the specified amount.
+
@item original_size
Specify the size of the original video, the video for which the ASS file
was composed. For the syntax of this option, check the
@@ -17991,6 +17994,11 @@ To make the subtitles stream from @file{sub.srt}
appear in 80% transparent blue
subtitles=sub.srt:force_style='Fontname=DejaVu Serif,PrimaryColour='
@end example
+To re-sync subtitles after seeking the input e.g. with @code{-ss 20:20}, use:
+@example
+subtitles=filename=sub.srt:shift='-20\:20'
+@end example
+
@section super2xsai
Scale the input by 2x and smooth using the Super2xSaI (Scale and
diff --git a/libavfilter/vf_subtitles.c b/libavfilter/vf_subtitles.c
index 1bd42391e0..125fbd9ac7 100644
--- a/libavfilter/vf_subtitles.c
+++ b/libavfilter/vf_subtitles.c
@@ -52,6 +52,7 @@ typedef struct AssContext {
char *filename;
char *fontsdir;
char *charenc;
+int64_t shift;
char *force_style;
int stream_index;
int alpha;
@@ -103,6 +104,11 @@ static av_cold int init(AVFilterContext *ctx)
return AVERROR(EINVAL);
}
+if (ass->shift != 0) {
+ass->shift = av_rescale_q(ass->shift, AV_TIME_BASE_Q, av_make_q(1,
1000));
+av_log(ctx, AV_LOG_DEBUG, "Shifting subtitles by %0.3fsec.\n",
ass->shift/1000.0);
+}
+
ass->library = ass_library_init();
if (!ass->library) {
av_log(ctx, AV_LOG_ERROR, "Could not initialize libass.\n");
@@ -267,6 +273,7 @@ static const AVOption subtitles_options[] = {
{"stream_index", "set stream index", OFFSET(stream_index),
AV_OPT_TYPE_INT,{ .i64 = -1 }, -1, INT_MAX, FLAGS},
{"si", "set stream index", OFFSET(stream_index),
AV_OPT_TYPE_INT,{ .i64 = -1 }, -1, INT_MAX, FLAGS},
{"force_style", "force subtitle style", OFFSET(force_style),
AV_OPT_TYPE_STRING, {.str = NULL}, 0, 0, FLAGS},
+{"shift","shift subtitles timing", OFFSET(shift),
AV_OPT_TYPE_DURATION, {.i64 = 0}, INT64_MIN, INT64_MAX, FLAGS },
{NULL},
};
@@ -297,7 +304,7 @@ AVFILTER_DEFINE_CLASS(subtitles);
static av_cold int init_subtitles(AVFilterContext *ctx)
{
-int j, ret, sid;
+int j, ret, sid, nskip;
int k = 0;
AVDictionary *codec_opts = NULL;
AVFormatContext *fmt = NULL;
@@ -448,6 +455,7 @@ static av_cold int init_subtitles(AVFilterContext *ctx)
av_init_packet();
pkt.data = NULL;
pkt.size = 0;
+nskip = 0;
while (av_read_frame(fmt, ) >= 0) {
int i, got_subtitle;
AVSubtitle sub = {0};
@@ -458,8 +466,17 @@ static av_cold int init_subtitles(AVFilterContext *ctx)
av_log(ctx, AV_LOG_WARNING, "Error decoding: %s (ignored)\n",
av_err2str(ret));
} else if (got_subtitle) {
-const int64_t start_time = av_rescale_q(sub.pts,
AV_TIME_BASE_Q, av_make_q(1, 1000));
+const int64_t start_time = av_rescale_q(sub.pts,
AV_TIME_BASE_Q, av_make_q(1, 1000)) + ass->shift;
const int64_t duration = sub.end_display_time;
+
+if (start_time + duration < 0) {
+nskip++;
+goto pkt_end;
+} else if (nskip > 0) {
+av_log(ctx, AV_LOG_INFO, "Skipped %d subtitles out of time
range.\n", nskip);
+nskip = 0;
+}
+
for (i = 0; i < sub.num_rects; i++) {
char *ass_line = sub.rects[i]->ass;
if (!ass_line)
@@ -472,6 +489,8 @@ static av_cold int init_subtitles(AVFilterContext *ctx)
}
}
}
+
+pkt_end:
av_packet_unref();
avsubtitle_free();
}
--
2.17.1
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] patch submission questions
Thanks for the responses Hongcheng and Steinar! I'll use '-n' with git format-patch if I need to resubmit. Patches already work as Steinar describes. In the meantime patch 2/3 found its way to patchwork. The delay was probably due to some email processing hiccup. So all is good! Cheers, Manolis On Sun, 5 Jul 2020 at 16:06, Steinar H. Gunderson < steinar+ffm...@gunderson.no> wrote: > On Sun, Jul 05, 2020 at 03:42:34PM +0200, Manolis Stamatogiannakis wrote: > > Q2: In a patchset consisting of several commits, is each commit expected > to > > be "standalone"? I.e. does it have to apply cleanly without depending on > > the previous commits in the patchset? > > No, but it has to compile and work even if not applying any of the latter > patches. > (This increases reviewability and bisectability.) AFAIK Patchwork doesn't > enforce this, though. > > /* Steinar */ > -- > Homepage: https://www.sesse.net/ > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe". ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH v2 3/3] doc/developer.texi: Swapped patch checklist and new codec/format checklist.
Adding a new codec/format should be more rare, so it makes sense
to come after the detailed patch submission checklist.
Signed-off-by: Manolis Stamatogiannakis
---
doc/developer.texi | 105 ++---
1 file changed, 52 insertions(+), 53 deletions(-)
diff --git a/doc/developer.texi b/doc/developer.texi
index c47d23e349..c482c5e09d 100644
--- a/doc/developer.texi
+++ b/doc/developer.texi
@@ -535,59 +535,6 @@ its @emph{roll-counter}. This is achieved by adding a
@code{-v } argument
to @code{git format-patch}/@code{git send-email} commands. While this is not
a strict requirement, it is a commonly followed good practice.
-@anchor{new codec format checklist}
-@section New codecs or formats checklist
-
-@enumerate
-@item
-Did you use av_cold for codec initialization and close functions?
-
-@item
-Did you add a long_name under NULL_IF_CONFIG_SMALL to the AVCodec or
-AVInputFormat/AVOutputFormat struct?
-
-@item
-Did you bump the minor version number (and reset the micro version
-number) in @file{libavcodec/version.h} or @file{libavformat/version.h}?
-
-@item
-Did you register it in @file{allcodecs.c} or @file{allformats.c}?
-
-@item
-Did you add the AVCodecID to @file{avcodec.h}?
-When adding new codec IDs, also add an entry to the codec descriptor
-list in @file{libavcodec/codec_desc.c}.
-
-@item
-If it has a FourCC, did you add it to @file{libavformat/riff.c},
-even if it is only a decoder?
-
-@item
-Did you add a rule to compile the appropriate files in the Makefile?
-Remember to do this even if you're just adding a format to a file that is
-already being compiled by some other rule, like a raw demuxer.
-
-@item
-Did you add an entry to the table of supported formats or codecs in
-@file{doc/general.texi}?
-
-@item
-Did you add an entry in the Changelog?
-
-@item
-If it depends on a parser or a library, did you add that dependency in
-configure?
-
-@item
-Did you @code{git add} the appropriate files before committing?
-
-@item
-Did you make sure it compiles standalone, i.e. with
-@code{configure --disable-everything --enable-decoder=foo}
-(or @code{--enable-demuxer} or whatever your component is)?
-@end enumerate
-
-
@anchor{patch submission checklist}
@section Patch submission checklist
@@ -710,6 +657,58 @@ Test your code with valgrind and or Address Sanitizer to
ensure it's free
of leaks, out of array accesses, etc.
@end enumerate
+@anchor{new codec format checklist}
+@section New codecs or formats checklist
+
+@enumerate
+@item
+Did you use av_cold for codec initialization and close functions?
+
+@item
+Did you add a long_name under NULL_IF_CONFIG_SMALL to the AVCodec or
+AVInputFormat/AVOutputFormat struct?
+
+@item
+Did you bump the minor version number (and reset the micro version
+number) in @file{libavcodec/version.h} or @file{libavformat/version.h}?
+
+@item
+Did you register it in @file{allcodecs.c} or @file{allformats.c}?
+
+@item
+Did you add the AVCodecID to @file{avcodec.h}?
+When adding new codec IDs, also add an entry to the codec descriptor
+list in @file{libavcodec/codec_desc.c}.
+
+@item
+If it has a FourCC, did you add it to @file{libavformat/riff.c},
+even if it is only a decoder?
+
+@item
+Did you add a rule to compile the appropriate files in the Makefile?
+Remember to do this even if you're just adding a format to a file that is
+already being compiled by some other rule, like a raw demuxer.
+
+@item
+Did you add an entry to the table of supported formats or codecs in
+@file{doc/general.texi}?
+
+@item
+Did you add an entry in the Changelog?
+
+@item
+If it depends on a parser or a library, did you add that dependency in
+configure?
+
+@item
+Did you @code{git add} the appropriate files before committing?
+
+@item
+Did you make sure it compiles standalone, i.e. with
+@code{configure --disable-everything --enable-decoder=foo}
+(or @code{--enable-demuxer} or whatever your component is)?
+@end enumerate
+
@chapter Patch review process
All patches posted to ffmpeg-devel will be reviewed, unless they contain a
--
2.17.1
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH v2 1/3] doc/developer.texi: Improvements in "Submitting patches" section.
- Documentation for adding a roll-counter.
- Mention the possibility to manualy supersede/withdraw patches on
patchwork.
Signed-off-by: Manolis Stamatogiannakis
---
doc/developer.texi | 41 +
1 file changed, 29 insertions(+), 12 deletions(-)
diff --git a/doc/developer.texi b/doc/developer.texi
index b33cab0fc7..585c37d241 100644
--- a/doc/developer.texi
+++ b/doc/developer.texi
@@ -491,18 +491,31 @@ as base64-encoded attachments, so your patch is not
trashed during
transmission. Also ensure the correct mime type is used
(text/x-diff or text/x-patch or at least text/plain) and that only one
patch is inline or attached per mail.
-You can check @url{https://patchwork.ffmpeg.org}, if your patch does not show
up, its mime type
-likely was wrong.
-
-Your patch will be reviewed on the mailing list. You will likely be asked
+You can check the most recently received patches on
+@url{https://patchwork.ffmpeg.org, patchwork}.
+If your patch does not show up, its mime type likely was wrong.
+
+If you need to manually manage your patch (e.g. urgently mark it as as
+"Superseded", or "Withdrawn") you can do so by
+@uref{https://patchwork.ffmpeg.org/register/, registering for a patchwork
account}.
+Note that this is not generally required, as patch management on patchwork
+is largely automated. But there are case where manual patch management is
+desired. For example, if you have started working on extensive changes
+on your patch, and you don't want people to spend more time reviewing the
+old version.
+
+Your patch will be reviewed on the mailing list. Give us a few days to react.
+But if some time passes without reaction, send a reminder by email.
+Your patch should eventually be dealt with. However, you will likely be asked
to make some changes and are expected to send in an improved version that
incorporates the requests from the review. This process may go through
several iterations. Once your patch is deemed good enough, some developer
will pick it up and commit it to the official FFmpeg tree.
-Give us a few days to react. But if some time passes without reaction,
-send a reminder by email. Your patch should eventually be dealt with.
-
+When preparing an updated version of you patch, you should increment
+its @emph{roll-counter}. This is achieved by adding a @code{-v } argument
+to @code{git format-patch}/@code{git send-email} commands. While this is not
+a strict requirement, it is a commonly followed good practice.
@chapter New codecs or formats checklist
@@ -563,7 +576,15 @@ Did you make sure it compiles standalone, i.e. with
Does @code{make fate} pass with the patch applied?
@item
-Was the patch generated with git format-patch or send-email?
+Was the patch generated with @code{git format-patch} or @code{git send-email}?
+
+@item
+If you are submitting a revised patch, did you increment the roll-counter
+with @code{-v }?
+
+@item
+Are you subscribed to ffmpeg-devel?
+(the list is subscribers only due to spam)
@item
Did you sign-off your patch? (@code{git commit -s})
@@ -576,10 +597,6 @@ Did you provide a clear git commit log message?
@item
Is the patch against latest FFmpeg git master branch?
-@item
-Are you subscribed to ffmpeg-devel?
-(the list is subscribers only due to spam)
-
@item
Have you checked that the changes are minimal, so that the same cannot be
achieved with a smaller patch and/or simpler final code?
--
2.17.1
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH v2 2/3] doc/developer.texi: Restructured "Submitting patches" section.
- Main text split to two sections.
- Detailed checklist for new codecs or formats demoted to section.
- Detailed checklist for patch submission demoted to section.
Signed-off-by: Manolis Stamatogiannakis
---
doc/developer.texi | 64 +++---
1 file changed, 44 insertions(+), 20 deletions(-)
diff --git a/doc/developer.texi b/doc/developer.texi
index 585c37d241..c47d23e349 100644
--- a/doc/developer.texi
+++ b/doc/developer.texi
@@ -457,31 +457,49 @@ Finally, keep in mind the immortal words of Bill and Ted,
@anchor{Submitting patches}
@chapter Submitting patches
-First, read the @ref{Coding Rules} above if you did not yet, in particular
+@anchor{patch guidelines}
+@section Guidelines for preparing a patch
+
+The @strong{absolute minimum} you have to do before submitting a patch are the
+following:
+
+@enumerate
+@item Carefully read the @ref{Coding Rules} above if you did not yet, in
particular
the rules regarding patch submission.
-When you submit your patch, please use @code{git format-patch} or
-@code{git send-email}. We cannot read other diffs :-).
+@item Make sure your commit messages accurately describe the changes made
+(e.g. 'replaces lrint by lrintf') and why these changes are made (e.g.
+'*BSD isn't C99 compliant and has no lrint()').
-Also please do not submit a patch which contains several unrelated changes.
-Split it into separate, self-contained pieces. This does not mean splitting
-file by file. Instead, make the patch as small as possible while still
-keeping it as a logical unit that contains an individual change, even
-if it spans multiple files. This makes reviewing your patches much easier
-for us and greatly increases your chances of getting your patch applied.
+@item Make sure you use @code{git format-patch} or @code{git send-email} to
prepare
+your patch. We cannot read other diffs :-).
+
+@item Run the @ref{Regression tests, regression tests} before submitting a
patch
+in order to verify it does not cause unexpected problems.
-Use the patcheck tool of FFmpeg to check your patch.
-The tool is located in the tools directory.
+@item If you send your patches with an external email client
+(i.e. not @code{git send-email}), make sure to send each patch as a separate
+email. Do not attach several patches to the same email!
-Run the @ref{Regression tests} before submitting a patch in order to verify
-it does not cause unexpected problems.
+@item Do not submit a patch which contains several unrelated changes.
+@end enumerate
+
+Additionally, it is also important that each patch is logically
+self-contained. I.e. each patch should be as small as possible,
+while still containing a meaningful individual change.
+Patches spanning multiple files are perfectly fine, as long as they can
+be seen as a single logical unit.
-It also helps quite a bit if you tell us what the patch does (for example
-'replaces lrint by lrintf'), and why (for example '*BSD isn't C99 compliant
-and has no lrint()')
+Following these guidelines makes reviewing your patches much easier
+for us and greatly increases your chances of getting your patch applied.
+To further reduce the chance that you will need to revise your patch,
+it is also recommended to go through the detailed
+@ref{patch submission checklist, patch} and
+@ref{new codec format checklist, new codec or format}
+checklists.
-Also please if you send several patches, send each patch as a separate mail,
-do not attach several unrelated patches to the same mail.
+@anchor{patch submission process}
+@section Patch submission and revision process
Patches should be posted to the
@uref{https://lists.ffmpeg.org/mailman/listinfo/ffmpeg-devel, ffmpeg-devel}
@@ -517,7 +535,8 @@ its @emph{roll-counter}. This is achieved by adding a
@code{-v } argument
to @code{git format-patch}/@code{git send-email} commands. While this is not
a strict requirement, it is a commonly followed good practice.
-@chapter New codecs or formats checklist
+@anchor{new codec format checklist}
+@section New codecs or formats checklist
@enumerate
@item
@@ -569,7 +588,8 @@ Did you make sure it compiles standalone, i.e. with
@end enumerate
-@chapter Patch submission checklist
+@anchor{patch submission checklist}
+@section Patch submission checklist
@enumerate
@item
@@ -594,6 +614,10 @@ of @dfn{sign-off}.
@item
Did you provide a clear git commit log message?
+@item
+Did you use the @code{patcheck} tool of FFmpeg to check your patch
+for common issues? E.g. @code{tools/patcheck *.patch}.
+
@item
Is the patch against latest FFmpeg git master branch?
--
2.17.1
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v1 1/3] doc/developer.texi: Improvements in "Submitting patches" section.
On Thu, 9 Jul 2020 at 22:39, Michael Niedermayer wrote: > > > @Michael Niedermayer, since you seem to be the most involved with > patchwork > > in the thread, what would be better for this? Keep the wording as a > > recommendation, or to move it outside the list as purely informational > text? > > I think there are 2 aspects here > First is a "mostly automatic patchwork with only people who want to play > with > it obtaining accounts and doing something with them" vs one where its > recommanded to get accounts > These are 2 different philosophies ;) > > Second is, time vs time > Is the gain from manually working with patchwork saving us more time > elsewhere while > we achieve the same quality of code? > > and Third > Can we not automate whatever we would do manually ? > > And describing the actual cases with examples what is wrong and needs > manual > update would be interresting. > > Are these caseses that are clear to human but unclear to a computer ? > or we are just missing a line or 2 in the script ? > or are they maybe unclear to humans too ? which could then lead > a discussion on how to make that clear to humans first ... > > I think its needed to understand where automation fails to really discuss > this all > > thx > > Thanks for the input Michael. If you find those benefits are not clear-cut, I agree we should refrain from recommending to everyone to create a patchwork account. I'm not intimate enough with patchwork automation, so my intuition on the benefits of recommending manual patch management may be off. I've updated the patch to v2, presenting the registration of a patchwork account as an optional step that may come handy in some cases, but not generally required. If anyone can make a more informed proposal on that matter, I can incorporate it in the patch, while we're looking at it. Best regards, Manolis ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v1 1/3] doc/developer.texi: Improvements in "Submitting patches" section.
On Tue, 7 Jul 2020 at 19:14, Michael Niedermayer wrote: > > a lot of patchwork patch maintaince can be automated, ive written a script > to do that. > What this does is basically look at local git, fetch all patches from > patchwork > (and cache them locally) > and then find stuff which is either invalid, superseeded, applied, ... but > not > marked correctly > last it dumps the command line commands to the screen for a human to decide > if they are correct and should be run as is or not. > See: > https://github.com/michaelni/patchwork-update-bot > > It worked fine until the server was updated or maybe the number of patches > became too big. > now it times out while fetching patches, the command line tool (pwclient) > times out too (xmlrpclib.ProtocolError: patchwork.ffmpeg.org/xmlrpc/: 504 Gateway Time-out>) > so the bug is not in patchwork-update-bot > Someone has to investigate why this timeout happens and make it not timeout > then run this automatically in regular intervalls > > thx > > Since it's python, I can take a look, provided it doesn't need any special permissions. M. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] Project orientation
On Tue, 7 Jul 2020 at 16:27, Nicolas George wrote: > Manolis Stamatogiannakis (12020-07-07): > > If I reply to the email, my response will appear online in the issue/PR > > page. > > That is good to know. I have never noticed it documented. Does it work > reliably? > > Haven't noticed any glitches so far. I can help you test the feature if you want. Just make a dummy public github repo and send me the details with email. > > Now, if you also want to review the code of a PRs from mutt, that's > another > > discussion. > > Not necessarily mutt, but if it's a web browser, then find somebody else > to review. > Code review on github is pretty nice, if you haven't tried it. > > Is tree threading that important? A PR is essentially a single thread of > > discussion. > > It is a single thread of discussion until the discussion becomes complex > and has branches. > This doesn't sound like the common case. But it should be straightforward to get some statistics on that from the list archives when a transition is officially discussed. > > > I would first ask why keep using a mailing list to post patches and not > use > > PRs. For a 3 commit patch, and 3 people sending feedback a few days > apart, > > you have a minimum of 15 emails for everyone on the list. Is that > efficient? > > Actually, since mail servers are much much less resource-consuming that > web interfaces, it probably is. > Valuing hardware resources more than the time any of us volunteers is a pretty unorthodox approach. We are not living in the 60s. Hardware is cheap, programmers are expensive [1]. [1] https://blog.codinghorror.com/hardware-is-cheap-programmers-are-expensive/ Best regards, Manolis ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v1 1/3] doc/developer.texi: Improvements in "Submitting patches" section.
On Tue, 7 Jul 2020 at 16:00, Nicolas George wrote:
> Manolis Stamatogiannakis (12020-07-05):
> > The section has been expanded to outline how to manage patch revisions.
> > ---
> > doc/developer.texi | 37 ++---
> > 1 file changed, 26 insertions(+), 11 deletions(-)
> >
> > diff --git a/doc/developer.texi b/doc/developer.texi
> > index b33cab0fc7..dec27cb509 100644
> > --- a/doc/developer.texi
> > +++ b/doc/developer.texi
> > @@ -491,18 +491,25 @@ as base64-encoded attachments, so your patch is
> not trashed during
> > transmission. Also ensure the correct mime type is used
> > (text/x-diff or text/x-patch or at least text/plain) and that only one
> > patch is inline or attached per mail.
> > -You can check @url{https://patchwork.ffmpeg.org}, if your patch does
> not show up, its mime type
> > -likely was wrong.
> > +You can check the most recently received patches on
> > +@url{https://patchwork.ffmpeg.org, patchwork}.
> > +If your patch does not show up, its mime type likely was wrong.
> >
> > -Your patch will be reviewed on the mailing list. You will likely be
> asked
> > +Your patch will be reviewed on the mailing list. Give us a few days to
> react.
> > +But if some time passes without reaction, send a reminder by email.
> > +Your patch should eventually be dealt with. However, you will likely be
> asked
> > to make some changes and are expected to send in an improved version
> that
> > incorporates the requests from the review. This process may go through
> > several iterations. Once your patch is deemed good enough, some
> developer
> > will pick it up and commit it to the official FFmpeg tree.
> >
> > -Give us a few days to react. But if some time passes without reaction,
> > -send a reminder by email. Your patch should eventually be dealt with.
> > -
>
> > +When preparing an updated version of you patch, make sure you increment
> > +its @emph{roll-counter}. This is achieved by adding a @code{-v }
> argument
> > +to @code{git format-patch}/@code{git send-email} commands.
>
> I know many core developers do not bother with that, and I never found
> these "v3" very useful: if I am not involved in the discussion, I will
> not remember what number is the latest. And if I become involved in the
> discussions, my mail client can sort the patch by date and I take the
> latest.
>
The documentation aims to establish some good practices for people who want
to start contributing to the project.
By the patches arriving to the list, adding a roll counter seems to be an
established good practice. So it should probably be included in the docs.
But keep in mind that this is only a recommendation anyway. Maybe changing
"make sure" to "should" would make that more clear.
Core developers can stray off recommendations, since they are in position
to make an informed decision when they are not relevant.
In this case, you have a very well setup email-based workflow. Other people
may not have that, and the roll counter may come handy.
Would normalizing the verbs in developer.texi help to avoid confusion?
Maybe it's just me, but when I'm reading any technical documents, I
interpret the verbs according to RFC2119 to understand what is mandatory,
what is recommended and what optional.
>
> > +Additionally, it is recommended to register for a
> > +@uref{https://patchwork.ffmpeg.org, patchwork account}.
> > +This will allow you to mark previous version of your patches as
> "Superseded",
> > +and reduce the chance of someone spending time to review a stale patch.
>
> Is interacting with Patchwork to become mandatory when submitting
> patches?
>
Nicolas, why would you ever interpret "recommended" as mandatory? I think
it's as clear as it gets that it is not mandatory.
Again, this is a good practice which was not mentioned in the
documentation. I have no attachment whatsoever to the tool, or intention to
impose it to anyone.
@Michael Niedermayer, since you seem to be the most involved with patchwork
in the thread, what would be better for this? Keep the wording as a
recommendation, or to move it outside the list as purely informational text?
> There is this classic scenario:
>
> 1. People work on something.
>
> 2. Somebody sets up a tool to make the work easier.
>
> 3. People start relying on the tool.
>
> 4. The tool proves imperfect.
>
> 5. People are required extra work to go around the flaws of the tool.
>
> 6. Overall, the tool has made the work not easier but harder.
>
> Are we going that way with Patchwork?
>
So your recommendation is to not setup any tool ever, because people may
Re: [FFmpeg-devel] [PATCH 3/3] avfilter/vf_subtitles: Reorganized subtitles filter options.
Fair point. I've submitted a v2 where the docs reorganization is applied
first.
On Sun, 5 Jul 2020 at 17:30, Gyan Doshi wrote:
>
>
> On 05-07-2020 06:35 pm, Manolis Stamatogiannakis wrote:
> > Some options are common between subtitles/ass filters.
> > Rather than mentioning for each option whether it is common or not,
> > the options are now displayed in two separate tables.
> >
> > Signed-off-by: Manolis Stamatogiannakis
> > ---
> > doc/filters.texi | 18 +++---
> > 1 file changed, 11 insertions(+), 7 deletions(-)
> >
> > diff --git a/doc/filters.texi b/doc/filters.texi
> > index c962ac55b0..c4ca39cb6d 100644
> > --- a/doc/filters.texi
> > +++ b/doc/filters.texi
> > @@ -6453,6 +6453,7 @@ This filter supports the following @ref{commands}
> that corresponds to option of
> > @item planes
> > @end table
> >
> > +@anchor{ass}
> > @section ass
> >
> > Same as the @ref{subtitles} filter, except that it doesn't require
> libavcodec
> > @@ -17929,15 +17930,12 @@ To enable compilation of this filter you need
> to configure FFmpeg with
> > libavformat to convert the passed subtitles file to ASS (Advanced
> Substation
> > Alpha) subtitles format.
> >
> > -The filter accepts the following options:
> > +Common @ref{subtitles}/@ref{ass} filter options:
> >
> > @table @option
> > @item filename, f
> > Set the filename of the subtitle file to read. It must be specified.
> >
> > -@item shift
> > -Shift subtitles timings by the specified amount.
> > -
> > @item original_size
> > Specify the size of the original video, the video for which the ASS
> file
> > was composed. For the syntax of this option, check the
> > @@ -17952,12 +17950,18 @@ These fonts will be used in addition to
> whatever the font provider uses.
> > @item alpha
> > Process alpha channel, by default alpha channel is untouched.
> >
> > +@item shift
> > +Shift subtitles timings by the specified amount.
> > +@end table
> > +
> > +Additional options for @ref{subtitles} filter:
> > +
> > +@table @option
> > @item charenc
> > -Set subtitles input character encoding. @code{subtitles} filter only.
> Only
> > -useful if not UTF-8.
> > +Set subtitles input character encoding. Only useful if not UTF-8.
> >
> > @item stream_index, si
> > -Set subtitles stream index. @code{subtitles} filter only.
> > +Set subtitles stream index.
>
> Break this off into a standalone without the shift option entry.
> Then merge the doc shift entry with the code patches.
>
> 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-requ...@ffmpeg.org with subject "unsubscribe".
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] Project orientation
On Sun, 5 Jul 2020 at 19:01, Kieran Kunhya wrote: > > For new contributors git send-email is annoying. For people wanting to > push, the .mbox format is annoying, Gmail doesn't support it any more. > And you can't get new contributors to start using CLI based email > clients or run their own mail server, that's not going to happen. > As a fresh contributor, setting up git send-email was a hassle, but not an insurmountable obstacle. Though I'd argue that git send-email is a bigger liability for regular developers. A lot of developers seem to be using Gmail, which means they need to have "less secure apps" enabled at the time they use git send-email. So they are forced to choose between security and convenience. I.e. having to temporarily enable "less secure apps" every time they submit, or accept the risk of weakened security for their account. > > A solution like Gitlab is the only way forward. It has worked well for > dav1d, it can run regression tests on all platforms for all commits: > https://code.videolan.org/videolan/dav1d > > Merges are done with one push of a button. Yes, the branch sprawl is > not great but it's better than now. > It has inline patch reviews which are nice. > FWIW, I'd add that branch-based PRs as implemented with GitHub greatly reduce the turnaround for receiving feedback and addressing the raised issues. All you have to do is force-push on your PR branch, and the PR is cleanly updated. This is in contrast with having to use git send-email, where the emails containing the stale patches will continue to litter patchwork and your mailboxes. Also, IIRC, when you update a branch, the regression tests for all pending PRs on the branch are automatically rerun. This should help address the conflicts in the pending PRs sooner rather than later. Not sure what the status of GitLab is wrt to these features, but I'd expect it to not be very far behind. Best regards, Manolis ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH 2/2] avfilter/vf_subtitles: Added shift option for subtitles/ass filters.
Allows shifting of subtitle display times to align them with the video.
This avoids having to rewrite the subtitle file in order to display
subtitles correctly when input is seeked (-ss).
Also handy for minor subtitle timing corrections without rewriting the
subtitles file.
Signed-off-by: Manolis Stamatogiannakis
---
doc/filters.texi | 11
libavfilter/vf_subtitles.c | 55 +-
2 files changed, 59 insertions(+), 7 deletions(-)
diff --git a/doc/filters.texi b/doc/filters.texi
index d2b8feb14b..a8af563551 100644
--- a/doc/filters.texi
+++ b/doc/filters.texi
@@ -17936,6 +17936,9 @@ Common @ref{subtitles}/@ref{ass} filter options:
@item filename, f
Set the filename of the subtitle file to read. It must be specified.
+@item shift
+Shift subtitles timings by the specified amount.
+
@item original_size
Specify the size of the original video, the video for which the ASS file
was composed. For the syntax of this option, check the
@@ -17949,6 +17952,9 @@ These fonts will be used in addition to whatever the
font provider uses.
@item alpha
Process alpha channel, by default alpha channel is untouched.
+
+@item shift
+Shift subtitles timings by the specified amount.
@end table
Additional options for @ref{subtitles} filter:
@@ -17995,6 +18001,11 @@ To make the subtitles stream from @file{sub.srt}
appear in 80% transparent blue
subtitles=sub.srt:force_style='Fontname=DejaVu Serif,PrimaryColour='
@end example
+To re-sync subtitles after seeking the input e.g. with @code{-ss 20:20}, use:
+@example
+subtitles=filename=sub.srt:shift='-20\:20'
+@end example
+
@section super2xsai
Scale the input by 2x and smooth using the Super2xSaI (Scale and
diff --git a/libavfilter/vf_subtitles.c b/libavfilter/vf_subtitles.c
index 1bd42391e0..af1bc9cd18 100644
--- a/libavfilter/vf_subtitles.c
+++ b/libavfilter/vf_subtitles.c
@@ -52,6 +52,7 @@ typedef struct AssContext {
char *filename;
char *fontsdir;
char *charenc;
+int64_t shift;
char *force_style;
int stream_index;
int alpha;
@@ -66,11 +67,12 @@ typedef struct AssContext {
#define FLAGS AV_OPT_FLAG_FILTERING_PARAM|AV_OPT_FLAG_VIDEO_PARAM
#define COMMON_OPTIONS \
-{"filename", "set the filename of file to read",
OFFSET(filename), AV_OPT_TYPE_STRING, {.str = NULL}, 0, 0, FLAGS }, \
-{"f", "set the filename of file to read",
OFFSET(filename), AV_OPT_TYPE_STRING, {.str = NULL}, 0, 0, FLAGS }, \
-{"original_size", "set the size of the original video (used to scale
fonts)", OFFSET(original_w), AV_OPT_TYPE_IMAGE_SIZE, {.str = NULL}, 0, 0,
FLAGS }, \
-{"fontsdir", "set the directory containing the fonts to read",
OFFSET(fontsdir), AV_OPT_TYPE_STRING, {.str = NULL}, 0, 0, FLAGS }, \
-{"alpha", "enable processing of alpha channel",
OFFSET(alpha), AV_OPT_TYPE_BOOL, {.i64 = 0 }, 0,
1, FLAGS }, \
+{"filename", "set the filename of file to read",
OFFSET(filename), AV_OPT_TYPE_STRING, {.str = NULL}, 0,
0, FLAGS }, \
+{"f", "set the filename of file to read",
OFFSET(filename), AV_OPT_TYPE_STRING, {.str = NULL}, 0,
0, FLAGS }, \
+{"original_size", "set the size of the original video (used to scale
fonts)", OFFSET(original_w), AV_OPT_TYPE_IMAGE_SIZE, {.str = NULL}, 0,
0, FLAGS }, \
+{"fontsdir", "set the directory containing the fonts to read",
OFFSET(fontsdir), AV_OPT_TYPE_STRING, {.str = NULL}, 0,
0, FLAGS }, \
+{"alpha", "enable processing of alpha channel",
OFFSET(alpha), AV_OPT_TYPE_BOOL, {.i64 = 0 }, 0,
1, FLAGS }, \
+{"shift", "shift subtitles timing",
OFFSET(shift), AV_OPT_TYPE_DURATION, {.i64 = 0 }, INT64_MIN,
INT64_MAX, FLAGS }, \
/* libass supports a log level ranging from 0 to 7 */
static const int ass_libavfilter_log_level_map[] = {
@@ -103,6 +105,11 @@ static av_cold int init(AVFilterContext *ctx)
return AVERROR(EINVAL);
}
+if (ass->shift != 0) {
+ass->shift = av_rescale_q(ass->shift, AV_TIME_BASE_Q, av_make_q(1,
1000));
+av_log(ctx, AV_LOG_INFO, "Shifting subtitles by %0.3fsec.\n",
ass->shift/1000.0);
+}
+
ass->library = ass_library_init();
if (!ass->library) {
av_log(ctx, AV_LOG_ERROR, "Could not initialize libass.\n");
@@ -227,6 +234,8 @@ A
[FFmpeg-devel] [PATCH 2/2] avfilter/vf_subtitles: Added shift option for subtitles/ass filters.
Allows shifting of subtitle display times to align them with the video.
This avoids having to rewrite the subtitle file in order to display
subtitles correctly when input is seeked (-ss).
Also handy for minor subtitle timing corrections without rewriting the
subtitles file.
Signed-off-by: Manolis Stamatogiannakis
---
doc/filters.texi | 11
libavfilter/vf_subtitles.c | 54 +-
2 files changed, 58 insertions(+), 7 deletions(-)
diff --git a/doc/filters.texi b/doc/filters.texi
index d2b8feb14b..a8af563551 100644
--- a/doc/filters.texi
+++ b/doc/filters.texi
@@ -17936,6 +17936,9 @@ Common @ref{subtitles}/@ref{ass} filter options:
@item filename, f
Set the filename of the subtitle file to read. It must be specified.
+@item shift
+Shift subtitles timings by the specified amount.
+
@item original_size
Specify the size of the original video, the video for which the ASS file
was composed. For the syntax of this option, check the
@@ -17949,6 +17952,9 @@ These fonts will be used in addition to whatever the
font provider uses.
@item alpha
Process alpha channel, by default alpha channel is untouched.
+
+@item shift
+Shift subtitles timings by the specified amount.
@end table
Additional options for @ref{subtitles} filter:
@@ -17995,6 +18001,11 @@ To make the subtitles stream from @file{sub.srt}
appear in 80% transparent blue
subtitles=sub.srt:force_style='Fontname=DejaVu Serif,PrimaryColour='
@end example
+To re-sync subtitles after seeking the input e.g. with @code{-ss 20:20}, use:
+@example
+subtitles=filename=sub.srt:shift='-20\:20'
+@end example
+
@section super2xsai
Scale the input by 2x and smooth using the Super2xSaI (Scale and
diff --git a/libavfilter/vf_subtitles.c b/libavfilter/vf_subtitles.c
index 1bd42391e0..09cca6aab8 100644
--- a/libavfilter/vf_subtitles.c
+++ b/libavfilter/vf_subtitles.c
@@ -52,6 +52,7 @@ typedef struct AssContext {
char *filename;
char *fontsdir;
char *charenc;
+int64_t shift;
char *force_style;
int stream_index;
int alpha;
@@ -66,11 +67,12 @@ typedef struct AssContext {
#define FLAGS AV_OPT_FLAG_FILTERING_PARAM|AV_OPT_FLAG_VIDEO_PARAM
#define COMMON_OPTIONS \
-{"filename", "set the filename of file to read",
OFFSET(filename), AV_OPT_TYPE_STRING, {.str = NULL}, 0, 0, FLAGS }, \
-{"f", "set the filename of file to read",
OFFSET(filename), AV_OPT_TYPE_STRING, {.str = NULL}, 0, 0, FLAGS }, \
-{"original_size", "set the size of the original video (used to scale
fonts)", OFFSET(original_w), AV_OPT_TYPE_IMAGE_SIZE, {.str = NULL}, 0, 0,
FLAGS }, \
-{"fontsdir", "set the directory containing the fonts to read",
OFFSET(fontsdir), AV_OPT_TYPE_STRING, {.str = NULL}, 0, 0, FLAGS }, \
-{"alpha", "enable processing of alpha channel",
OFFSET(alpha), AV_OPT_TYPE_BOOL, {.i64 = 0 }, 0,
1, FLAGS }, \
+{"filename", "set the filename of file to read",
OFFSET(filename), AV_OPT_TYPE_STRING, {.str = NULL}, 0,
0, FLAGS }, \
+{"f", "set the filename of file to read",
OFFSET(filename), AV_OPT_TYPE_STRING, {.str = NULL}, 0,
0, FLAGS }, \
+{"original_size", "set the size of the original video (used to scale
fonts)", OFFSET(original_w), AV_OPT_TYPE_IMAGE_SIZE, {.str = NULL}, 0,
0, FLAGS }, \
+{"fontsdir", "set the directory containing the fonts to read",
OFFSET(fontsdir), AV_OPT_TYPE_STRING, {.str = NULL}, 0,
0, FLAGS }, \
+{"alpha", "enable processing of alpha channel",
OFFSET(alpha), AV_OPT_TYPE_BOOL, {.i64 = 0 }, 0,
1, FLAGS }, \
+{"shift", "shift subtitles timing",
OFFSET(shift), AV_OPT_TYPE_DURATION, {.i64 = 0 }, INT64_MIN,
INT64_MAX, FLAGS }, \
/* libass supports a log level ranging from 0 to 7 */
static const int ass_libavfilter_log_level_map[] = {
@@ -103,6 +105,11 @@ static av_cold int init(AVFilterContext *ctx)
return AVERROR(EINVAL);
}
+if (ass->shift != 0) {
+ass->shift = av_rescale_q(ass->shift, AV_TIME_BASE_Q, av_make_q(1,
1000));
+av_log(ctx, AV_LOG_INFO, "Shifting subtitles by %0.3fsec.\n",
ass->shift/1000.0);
+}
+
ass->library = ass_library_init();
if (!ass->library) {
av_log(ctx, AV_LOG_ERROR, "Could not initialize libass.\n");
@@ -227,6 +234,8 @@ A
[FFmpeg-devel] [PATCH 1/2] avfilter/vf_subtitles: Reorganized subtitles filter options.
Some options are common between subtitles/ass filters.
Rather than mentioning for each option whether it is common or not,
the options are now displayed in two separate tables.
Signed-off-by: Manolis Stamatogiannakis
---
doc/filters.texi | 12
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/doc/filters.texi b/doc/filters.texi
index ad2448acb2..d2b8feb14b 100644
--- a/doc/filters.texi
+++ b/doc/filters.texi
@@ -6453,6 +6453,7 @@ This filter supports the following @ref{commands} that
corresponds to option of
@item planes
@end table
+@anchor{ass}
@section ass
Same as the @ref{subtitles} filter, except that it doesn't require libavcodec
@@ -17929,7 +17930,7 @@ To enable compilation of this filter you need to
configure FFmpeg with
libavformat to convert the passed subtitles file to ASS (Advanced Substation
Alpha) subtitles format.
-The filter accepts the following options:
+Common @ref{subtitles}/@ref{ass} filter options:
@table @option
@item filename, f
@@ -17948,13 +17949,16 @@ These fonts will be used in addition to whatever the
font provider uses.
@item alpha
Process alpha channel, by default alpha channel is untouched.
+@end table
+Additional options for @ref{subtitles} filter:
+
+@table @option
@item charenc
-Set subtitles input character encoding. @code{subtitles} filter only. Only
-useful if not UTF-8.
+Set subtitles input character encoding. Only useful if not UTF-8.
@item stream_index, si
-Set subtitles stream index. @code{subtitles} filter only.
+Set subtitles stream index.
@item force_style
Override default style or script info parameters of the subtitles. It accepts a
--
2.17.1
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH 1/2] avfilter/vf_subtitles: Reorganized subtitles filter options.
Some options are common between subtitles/ass filters.
Rather than mentioning for each option whether it is common or not,
the options are now displayed in two separate tables.
Signed-off-by: Manolis Stamatogiannakis
---
doc/filters.texi | 12
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/doc/filters.texi b/doc/filters.texi
index ad2448acb2..d2b8feb14b 100644
--- a/doc/filters.texi
+++ b/doc/filters.texi
@@ -6453,6 +6453,7 @@ This filter supports the following @ref{commands} that
corresponds to option of
@item planes
@end table
+@anchor{ass}
@section ass
Same as the @ref{subtitles} filter, except that it doesn't require libavcodec
@@ -17929,7 +17930,7 @@ To enable compilation of this filter you need to
configure FFmpeg with
libavformat to convert the passed subtitles file to ASS (Advanced Substation
Alpha) subtitles format.
-The filter accepts the following options:
+Common @ref{subtitles}/@ref{ass} filter options:
@table @option
@item filename, f
@@ -17948,13 +17949,16 @@ These fonts will be used in addition to whatever the
font provider uses.
@item alpha
Process alpha channel, by default alpha channel is untouched.
+@end table
+Additional options for @ref{subtitles} filter:
+
+@table @option
@item charenc
-Set subtitles input character encoding. @code{subtitles} filter only. Only
-useful if not UTF-8.
+Set subtitles input character encoding. Only useful if not UTF-8.
@item stream_index, si
-Set subtitles stream index. @code{subtitles} filter only.
+Set subtitles stream index.
@item force_style
Override default style or script info parameters of the subtitles. It accepts a
--
2.17.1
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] patch submission questions
On Sun, 5 Jul 2020 at 15:48, Hongcheng Zhong wrote: > > You can use `git format-patch -v -n` to get patches like [PATCH > v2 1/20]. See git-format-patch documentation for more details. > > That didn't quite work. I used "git format-patch -s -n -v3 --to ffmpeg-devel@ffmpeg.org HEAD~2..HEAD" and "git send-email -2". But the -v argument was dropped and the end results on patchwork were again: [FFmpeg-devel,2/2] avfilter/vf_subtitles: Added shift option for subtitles/ass filters. [FFmpeg-devel,1/2] avfilter/vf_subtitles: Reorganized subtitles filter options. Manolis ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH 3/3] avfilter/vf_subtitles: Reorganized subtitles filter options.
Some options are common between subtitles/ass filters.
Rather than mentioning for each option whether it is common or not,
the options are now displayed in two separate tables.
Signed-off-by: Manolis Stamatogiannakis
---
doc/filters.texi | 18 +++---
1 file changed, 11 insertions(+), 7 deletions(-)
diff --git a/doc/filters.texi b/doc/filters.texi
index c962ac55b0..c4ca39cb6d 100644
--- a/doc/filters.texi
+++ b/doc/filters.texi
@@ -6453,6 +6453,7 @@ This filter supports the following @ref{commands} that
corresponds to option of
@item planes
@end table
+@anchor{ass}
@section ass
Same as the @ref{subtitles} filter, except that it doesn't require libavcodec
@@ -17929,15 +17930,12 @@ To enable compilation of this filter you need to
configure FFmpeg with
libavformat to convert the passed subtitles file to ASS (Advanced Substation
Alpha) subtitles format.
-The filter accepts the following options:
+Common @ref{subtitles}/@ref{ass} filter options:
@table @option
@item filename, f
Set the filename of the subtitle file to read. It must be specified.
-@item shift
-Shift subtitles timings by the specified amount.
-
@item original_size
Specify the size of the original video, the video for which the ASS file
was composed. For the syntax of this option, check the
@@ -17952,12 +17950,18 @@ These fonts will be used in addition to whatever the
font provider uses.
@item alpha
Process alpha channel, by default alpha channel is untouched.
+@item shift
+Shift subtitles timings by the specified amount.
+@end table
+
+Additional options for @ref{subtitles} filter:
+
+@table @option
@item charenc
-Set subtitles input character encoding. @code{subtitles} filter only. Only
-useful if not UTF-8.
+Set subtitles input character encoding. Only useful if not UTF-8.
@item stream_index, si
-Set subtitles stream index. @code{subtitles} filter only.
+Set subtitles stream index.
@item force_style
Override default style or script info parameters of the subtitles. It accepts a
--
2.17.1
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH 1/3] avfilter/vf_subtitles: add shift option
Allows shifting of subtitle display times to align them with the video.
This avoids having to rewrite the subtitle file in order to display
subtitles correctly when input is seeked (-ss).
Also handy for minor subtitle timing corrections without rewriting the
subtitles file.
Signed-off-by: Manolis Stamatogiannakis
---
doc/filters.texi | 8
libavfilter/vf_subtitles.c | 23 +--
2 files changed, 29 insertions(+), 2 deletions(-)
diff --git a/doc/filters.texi b/doc/filters.texi
index ad2448acb2..c962ac55b0 100644
--- a/doc/filters.texi
+++ b/doc/filters.texi
@@ -17935,6 +17935,9 @@ The filter accepts the following options:
@item filename, f
Set the filename of the subtitle file to read. It must be specified.
+@item shift
+Shift subtitles timings by the specified amount.
+
@item original_size
Specify the size of the original video, the video for which the ASS file
was composed. For the syntax of this option, check the
@@ -17991,6 +17994,11 @@ To make the subtitles stream from @file{sub.srt}
appear in 80% transparent blue
subtitles=sub.srt:force_style='Fontname=DejaVu Serif,PrimaryColour='
@end example
+To re-sync subtitles after seeking the input e.g. with @code{-ss 20:20}, use:
+@example
+subtitles=filename=sub.srt:shift='-20\:20'
+@end example
+
@section super2xsai
Scale the input by 2x and smooth using the Super2xSaI (Scale and
diff --git a/libavfilter/vf_subtitles.c b/libavfilter/vf_subtitles.c
index 1bd42391e0..125fbd9ac7 100644
--- a/libavfilter/vf_subtitles.c
+++ b/libavfilter/vf_subtitles.c
@@ -52,6 +52,7 @@ typedef struct AssContext {
char *filename;
char *fontsdir;
char *charenc;
+int64_t shift;
char *force_style;
int stream_index;
int alpha;
@@ -103,6 +104,11 @@ static av_cold int init(AVFilterContext *ctx)
return AVERROR(EINVAL);
}
+if (ass->shift != 0) {
+ass->shift = av_rescale_q(ass->shift, AV_TIME_BASE_Q, av_make_q(1,
1000));
+av_log(ctx, AV_LOG_DEBUG, "Shifting subtitles by %0.3fsec.\n",
ass->shift/1000.0);
+}
+
ass->library = ass_library_init();
if (!ass->library) {
av_log(ctx, AV_LOG_ERROR, "Could not initialize libass.\n");
@@ -267,6 +273,7 @@ static const AVOption subtitles_options[] = {
{"stream_index", "set stream index", OFFSET(stream_index),
AV_OPT_TYPE_INT,{ .i64 = -1 }, -1, INT_MAX, FLAGS},
{"si", "set stream index", OFFSET(stream_index),
AV_OPT_TYPE_INT,{ .i64 = -1 }, -1, INT_MAX, FLAGS},
{"force_style", "force subtitle style", OFFSET(force_style),
AV_OPT_TYPE_STRING, {.str = NULL}, 0, 0, FLAGS},
+{"shift","shift subtitles timing", OFFSET(shift),
AV_OPT_TYPE_DURATION, {.i64 = 0}, INT64_MIN, INT64_MAX, FLAGS },
{NULL},
};
@@ -297,7 +304,7 @@ AVFILTER_DEFINE_CLASS(subtitles);
static av_cold int init_subtitles(AVFilterContext *ctx)
{
-int j, ret, sid;
+int j, ret, sid, nskip;
int k = 0;
AVDictionary *codec_opts = NULL;
AVFormatContext *fmt = NULL;
@@ -448,6 +455,7 @@ static av_cold int init_subtitles(AVFilterContext *ctx)
av_init_packet();
pkt.data = NULL;
pkt.size = 0;
+nskip = 0;
while (av_read_frame(fmt, ) >= 0) {
int i, got_subtitle;
AVSubtitle sub = {0};
@@ -458,8 +466,17 @@ static av_cold int init_subtitles(AVFilterContext *ctx)
av_log(ctx, AV_LOG_WARNING, "Error decoding: %s (ignored)\n",
av_err2str(ret));
} else if (got_subtitle) {
-const int64_t start_time = av_rescale_q(sub.pts,
AV_TIME_BASE_Q, av_make_q(1, 1000));
+const int64_t start_time = av_rescale_q(sub.pts,
AV_TIME_BASE_Q, av_make_q(1, 1000)) + ass->shift;
const int64_t duration = sub.end_display_time;
+
+if (start_time + duration < 0) {
+nskip++;
+goto pkt_end;
+} else if (nskip > 0) {
+av_log(ctx, AV_LOG_INFO, "Skipped %d subtitles out of time
range.\n", nskip);
+nskip = 0;
+}
+
for (i = 0; i < sub.num_rects; i++) {
char *ass_line = sub.rects[i]->ass;
if (!ass_line)
@@ -472,6 +489,8 @@ static av_cold int init_subtitles(AVFilterContext *ctx)
}
}
}
+
+pkt_end:
av_packet_unref();
avsubtitle_free();
}
--
2.17.1
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] patch submission questions
Hello, I'm trying to submit a patch for adding a "shift" option to subtitles/ass filters. Initial submission was ok, but resubmitting after addressing some emails didn't go as expected. I have the following two questions on the process: Q1: How do you get the "v2" label when you resubmit a patch? I thought this would happen auto-magically, but it didn't. And I couldn't find anything relevant in https://ffmpeg.org/developer.html#Contributing Q2: In a patchset consisting of several commits, is each commit expected to be "standalone"? I.e. does it have to apply cleanly without depending on the previous commits in the patchset? Again this is not mentioned in the contributing guidelines, but my 2/3 patch didn't make it to patchwork.ffmpeg.org, and this seems like a plausible explanation. Thanks in advance, Manolis ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH 2/3] avfilter/vf_subtitles: add shift option for ass filter
Previously option implemented only for subtitles filter.
Signed-off-by: Manolis Stamatogiannakis
---
libavfilter/vf_subtitles.c | 35 ---
1 file changed, 28 insertions(+), 7 deletions(-)
diff --git a/libavfilter/vf_subtitles.c b/libavfilter/vf_subtitles.c
index 125fbd9ac7..09cca6aab8 100644
--- a/libavfilter/vf_subtitles.c
+++ b/libavfilter/vf_subtitles.c
@@ -67,11 +67,12 @@ typedef struct AssContext {
#define FLAGS AV_OPT_FLAG_FILTERING_PARAM|AV_OPT_FLAG_VIDEO_PARAM
#define COMMON_OPTIONS \
-{"filename", "set the filename of file to read",
OFFSET(filename), AV_OPT_TYPE_STRING, {.str = NULL}, 0, 0, FLAGS }, \
-{"f", "set the filename of file to read",
OFFSET(filename), AV_OPT_TYPE_STRING, {.str = NULL}, 0, 0, FLAGS }, \
-{"original_size", "set the size of the original video (used to scale
fonts)", OFFSET(original_w), AV_OPT_TYPE_IMAGE_SIZE, {.str = NULL}, 0, 0,
FLAGS }, \
-{"fontsdir", "set the directory containing the fonts to read",
OFFSET(fontsdir), AV_OPT_TYPE_STRING, {.str = NULL}, 0, 0, FLAGS }, \
-{"alpha", "enable processing of alpha channel",
OFFSET(alpha), AV_OPT_TYPE_BOOL, {.i64 = 0 }, 0,
1, FLAGS }, \
+{"filename", "set the filename of file to read",
OFFSET(filename), AV_OPT_TYPE_STRING, {.str = NULL}, 0,
0, FLAGS }, \
+{"f", "set the filename of file to read",
OFFSET(filename), AV_OPT_TYPE_STRING, {.str = NULL}, 0,
0, FLAGS }, \
+{"original_size", "set the size of the original video (used to scale
fonts)", OFFSET(original_w), AV_OPT_TYPE_IMAGE_SIZE, {.str = NULL}, 0,
0, FLAGS }, \
+{"fontsdir", "set the directory containing the fonts to read",
OFFSET(fontsdir), AV_OPT_TYPE_STRING, {.str = NULL}, 0,
0, FLAGS }, \
+{"alpha", "enable processing of alpha channel",
OFFSET(alpha), AV_OPT_TYPE_BOOL, {.i64 = 0 }, 0,
1, FLAGS }, \
+{"shift", "shift subtitles timing",
OFFSET(shift), AV_OPT_TYPE_DURATION, {.i64 = 0 }, INT64_MIN,
INT64_MAX, FLAGS }, \
/* libass supports a log level ranging from 0 to 7 */
static const int ass_libavfilter_log_level_map[] = {
@@ -106,7 +107,7 @@ static av_cold int init(AVFilterContext *ctx)
if (ass->shift != 0) {
ass->shift = av_rescale_q(ass->shift, AV_TIME_BASE_Q, av_make_q(1,
1000));
-av_log(ctx, AV_LOG_DEBUG, "Shifting subtitles by %0.3fsec.\n",
ass->shift/1000.0);
+av_log(ctx, AV_LOG_INFO, "Shifting subtitles by %0.3fsec.\n",
ass->shift/1000.0);
}
ass->library = ass_library_init();
@@ -233,6 +234,8 @@ AVFILTER_DEFINE_CLASS(ass);
static av_cold int init_ass(AVFilterContext *ctx)
{
+int eid, nskip;
+ASS_Event *event;
AssContext *ass = ctx->priv;
int ret = init(ctx);
@@ -249,6 +252,24 @@ static av_cold int init_ass(AVFilterContext *ctx)
ass->filename);
return AVERROR(EINVAL);
}
+
+/* Shift subtitles. */
+nskip = 0;
+for (eid = 0; eid < ass->track->n_events; eid++) {
+event = >track->events[eid];
+event->Start += ass->shift;
+if (event->Start + event->Duration < 0) {
+ass_free_event(ass->track, eid);
+nskip++;
+continue;
+} else if (nskip > 0) {
+av_log(ctx, AV_LOG_INFO, "Skipped %d subtitles out of time
range.\n", nskip);
+memmove(event - nskip, event, (ass->track->n_events - eid) *
sizeof(ASS_Event));
+ass->track->n_events -= nskip;
+nskip = 0;
+}
+}
+
return 0;
}
@@ -273,7 +294,6 @@ static const AVOption subtitles_options[] = {
{"stream_index", "set stream index", OFFSET(stream_index),
AV_OPT_TYPE_INT,{ .i64 = -1 }, -1, INT_MAX, FLAGS},
{"si", "set stream index", OFFSET(stream_index),
AV_OPT_TYPE_INT,{ .i64 = -1 }, -1, INT_MAX, FLAGS},
{"force_style", "force subtitle style", OFFSET(force_style),
AV_OPT_TYPE_STRING, {.str = NULL}, 0, 0, FLAGS},
-{"shift","shift subtitles timing", OFFSET(shift),
AV_OPT_TYPE_DURATION, {.i64 = 0}, INT64_MIN, INT64_MAX, FLAGS },
{NUL
[FFmpeg-devel] [PATCH v1 2/2] avfilter/vf_subtitles: Added shift option for subtitles/ass filters.
Allows shifting of subtitle display times to align them with the video.
This avoids having to rewrite the subtitle file in order to display
subtitles correctly when input is seeked (-ss).
Also handy for minor subtitle timing corrections without rewriting the
subtitles file.
Signed-off-by: Manolis Stamatogiannakis
---
doc/filters.texi | 11
libavfilter/vf_subtitles.c | 55 +-
2 files changed, 59 insertions(+), 7 deletions(-)
diff --git a/doc/filters.texi b/doc/filters.texi
index 61c4cfc150..eebf455692 100644
--- a/doc/filters.texi
+++ b/doc/filters.texi
@@ -19474,6 +19474,9 @@ Common @ref{subtitles}/@ref{ass} filter options:
@item filename, f
Set the filename of the subtitle file to read. It must be specified.
+@item shift
+Shift subtitles timings by the specified amount.
+
@item original_size
Specify the size of the original video, the video for which the ASS file
was composed. For the syntax of this option, check the
@@ -19487,6 +19490,9 @@ These fonts will be used in addition to whatever the
font provider uses.
@item alpha
Process alpha channel, by default alpha channel is untouched.
+
+@item shift
+Shift subtitles timings by the specified amount.
@end table
Additional options for @ref{subtitles} filter:
@@ -19533,6 +19539,11 @@ To make the subtitles stream from @file{sub.srt}
appear in 80% transparent blue
subtitles=sub.srt:force_style='Fontname=DejaVu Serif,PrimaryColour='
@end example
+To re-sync subtitles after seeking the input e.g. with @code{-ss 20:20}, use:
+@example
+subtitles=filename=sub.srt:shift='-20\:20'
+@end example
+
@section super2xsai
Scale the input by 2x and smooth using the Super2xSaI (Scale and
diff --git a/libavfilter/vf_subtitles.c b/libavfilter/vf_subtitles.c
index ab32e1b7f3..2c7ce267e1 100644
--- a/libavfilter/vf_subtitles.c
+++ b/libavfilter/vf_subtitles.c
@@ -52,6 +52,7 @@ typedef struct AssContext {
char *filename;
char *fontsdir;
char *charenc;
+int64_t shift;
char *force_style;
int stream_index;
int alpha;
@@ -66,11 +67,12 @@ typedef struct AssContext {
#define FLAGS AV_OPT_FLAG_FILTERING_PARAM|AV_OPT_FLAG_VIDEO_PARAM
#define COMMON_OPTIONS \
-{"filename", "set the filename of file to read",
OFFSET(filename), AV_OPT_TYPE_STRING, {.str = NULL}, 0, 0, FLAGS }, \
-{"f", "set the filename of file to read",
OFFSET(filename), AV_OPT_TYPE_STRING, {.str = NULL}, 0, 0, FLAGS }, \
-{"original_size", "set the size of the original video (used to scale
fonts)", OFFSET(original_w), AV_OPT_TYPE_IMAGE_SIZE, {.str = NULL}, 0, 0,
FLAGS }, \
-{"fontsdir", "set the directory containing the fonts to read",
OFFSET(fontsdir), AV_OPT_TYPE_STRING, {.str = NULL}, 0, 0, FLAGS }, \
-{"alpha", "enable processing of alpha channel",
OFFSET(alpha), AV_OPT_TYPE_BOOL, {.i64 = 0 }, 0,
1, FLAGS }, \
+{"filename", "set the filename of file to read",
OFFSET(filename), AV_OPT_TYPE_STRING, {.str = NULL}, 0,
0, FLAGS }, \
+{"f", "set the filename of file to read",
OFFSET(filename), AV_OPT_TYPE_STRING, {.str = NULL}, 0,
0, FLAGS }, \
+{"original_size", "set the size of the original video (used to scale
fonts)", OFFSET(original_w), AV_OPT_TYPE_IMAGE_SIZE, {.str = NULL}, 0,
0, FLAGS }, \
+{"fontsdir", "set the directory containing the fonts to read",
OFFSET(fontsdir), AV_OPT_TYPE_STRING, {.str = NULL}, 0,
0, FLAGS }, \
+{"alpha", "enable processing of alpha channel",
OFFSET(alpha), AV_OPT_TYPE_BOOL, {.i64 = 0 }, 0,
1, FLAGS }, \
+{"shift", "shift subtitles timing",
OFFSET(shift), AV_OPT_TYPE_DURATION, {.i64 = 0 }, INT64_MIN,
INT64_MAX, FLAGS }, \
/* libass supports a log level ranging from 0 to 7 */
static const int ass_libavfilter_log_level_map[] = {
@@ -103,6 +105,11 @@ static av_cold int init(AVFilterContext *ctx)
return AVERROR(EINVAL);
}
+if (ass->shift != 0) {
+ass->shift = av_rescale_q(ass->shift, AV_TIME_BASE_Q, av_make_q(1,
1000));
+av_log(ctx, AV_LOG_INFO, "Shifting subtitles by %0.3fsec.\n",
ass->shift/1000.0);
+}
+
ass->library = ass_library_init();
if (!ass->library) {
av_log(ctx, AV_LOG_ERROR, "Could not initialize libass.\n");
@@ -228,6 +235,8 @@ A
[FFmpeg-devel] [PATCH v1 1/2] avfilter/vf_subtitles: Reorganized subtitles filter options.
Some options are common between subtitles/ass filters.
Rather than mentioning for each option whether it is common or not,
the options are now displayed in two separate tables.
Signed-off-by: Manolis Stamatogiannakis
---
doc/filters.texi | 12
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/doc/filters.texi b/doc/filters.texi
index eaf23e3736..61c4cfc150 100644
--- a/doc/filters.texi
+++ b/doc/filters.texi
@@ -7093,6 +7093,7 @@ This filter supports the following @ref{commands} that
corresponds to option of
@item planes
@end table
+@anchor{ass}
@section ass
Same as the @ref{subtitles} filter, except that it doesn't require libavcodec
@@ -19467,7 +19468,7 @@ To enable compilation of this filter you need to
configure FFmpeg with
libavformat to convert the passed subtitles file to ASS (Advanced Substation
Alpha) subtitles format.
-The filter accepts the following options:
+Common @ref{subtitles}/@ref{ass} filter options:
@table @option
@item filename, f
@@ -19486,13 +19487,16 @@ These fonts will be used in addition to whatever the
font provider uses.
@item alpha
Process alpha channel, by default alpha channel is untouched.
+@end table
+Additional options for @ref{subtitles} filter:
+
+@table @option
@item charenc
-Set subtitles input character encoding. @code{subtitles} filter only. Only
-useful if not UTF-8.
+Set subtitles input character encoding. Only useful if not UTF-8.
@item stream_index, si
-Set subtitles stream index. @code{subtitles} filter only.
+Set subtitles stream index.
@item force_style
Override default style or script info parameters of the subtitles. It accepts a
--
2.17.1
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v1 2/2] avfilter/vf_subtitles: Added shift option for subtitles/ass filters.
Bumping this once more.
On Thu, 22 Jul 2021 at 16:19, Gyan Doshi wrote:
>
>
> On 2021-07-22 00:08, Manolis Stamatogiannakis wrote:
> > Would it be possible to have a quick review for this patch? It is pretty
> > straightforward.
>
> Will test within a few days.
>
> Regards,
> Gyan
>
> >
> > Plus, this is its second submission. It already includes the requested
> > changes from the first time (~1y ago).
> >
> > Thanks in advance,
> > Manolis
> >
> >
> > On Sun, 4 Jul 2021 at 18:13, Manolis Stamatogiannakis >
> > wrote:
> >
> >> Allows shifting of subtitle display times to align them with the video.
> >> This avoids having to rewrite the subtitle file in order to display
> >> subtitles correctly when input is seeked (-ss).
> >> Also handy for minor subtitle timing corrections without rewriting the
> >> subtitles file.
> >>
> >> Signed-off-by: Manolis Stamatogiannakis
> >> ---
> >> doc/filters.texi | 11
> >> libavfilter/vf_subtitles.c | 55 +-
> >> 2 files changed, 59 insertions(+), 7 deletions(-)
> >>
> >> diff --git a/doc/filters.texi b/doc/filters.texi
> >> index 61c4cfc150..eebf455692 100644
> >> --- a/doc/filters.texi
> >> +++ b/doc/filters.texi
> >> @@ -19474,6 +19474,9 @@ Common @ref{subtitles}/@ref{ass} filter options:
> >> @item filename, f
> >> Set the filename of the subtitle file to read. It must be specified.
> >>
> >> +@item shift
> >> +Shift subtitles timings by the specified amount.
> >> +
> >> @item original_size
> >> Specify the size of the original video, the video for which the ASS
> file
> >> was composed. For the syntax of this option, check the
> >> @@ -19487,6 +19490,9 @@ These fonts will be used in addition to whatever
> >> the font provider uses.
> >>
> >> @item alpha
> >> Process alpha channel, by default alpha channel is untouched.
> >> +
> >> +@item shift
> >> +Shift subtitles timings by the specified amount.
> >> @end table
> >>
> >> Additional options for @ref{subtitles} filter:
> >> @@ -19533,6 +19539,11 @@ To make the subtitles stream from
> @file{sub.srt}
> >> appear in 80% transparent blue
> >> subtitles=sub.srt:force_style='Fontname=DejaVu
> >> Serif,PrimaryColour='
> >> @end example
> >>
> >> +To re-sync subtitles after seeking the input e.g. with @code{-ss
> 20:20},
> >> use:
> >> +@example
> >> +subtitles=filename=sub.srt:shift='-20\:20'
> >> +@end example
> >> +
> >> @section super2xsai
> >>
> >> Scale the input by 2x and smooth using the Super2xSaI (Scale and
> >> diff --git a/libavfilter/vf_subtitles.c b/libavfilter/vf_subtitles.c
> >> index ab32e1b7f3..2c7ce267e1 100644
> >> --- a/libavfilter/vf_subtitles.c
> >> +++ b/libavfilter/vf_subtitles.c
> >> @@ -52,6 +52,7 @@ typedef struct AssContext {
> >> char *filename;
> >> char *fontsdir;
> >> char *charenc;
> >> +int64_t shift;
> >> char *force_style;
> >> int stream_index;
> >> int alpha;
> >> @@ -66,11 +67,12 @@ typedef struct AssContext {
> >> #define FLAGS AV_OPT_FLAG_FILTERING_PARAM|AV_OPT_FLAG_VIDEO_PARAM
> >>
> >> #define COMMON_OPTIONS \
> >> -{"filename", "set the filename of file to read",
> >> OFFSET(filename), AV_OPT_TYPE_STRING, {.str = NULL},
> 0, 0,
> >> FLAGS }, \
> >> -{"f", "set the filename of file to read",
> >> OFFSET(filename), AV_OPT_TYPE_STRING, {.str = NULL},
> 0, 0,
> >> FLAGS }, \
> >> -{"original_size", "set the size of the original video (used to
> scale
> >> fonts)", OFFSET(original_w), AV_OPT_TYPE_IMAGE_SIZE, {.str = NULL}, 0,
> 0,
> >> FLAGS }, \
> >> -{"fontsdir", "set the directory containing the fonts to
> read",
> >> OFFSET(fontsdir), AV_OPT_TYPE_STRING, {.str = NULL},
> 0, 0,
> >> FLAGS }, \
> >> -{"alpha", "enable processing of alpha channel",
> >> OFFSET(alpha), AV_OPT_TYPE_BOOL, {.i64 = 0 },
> >> 0,1, FLAGS }, \
> >> +{"file
Re: [FFmpeg-devel] [PATCH v1 2/2] avfilter/vf_subtitles: Added shift option for subtitles/ass filters.
Would it be possible to have a quick review for this patch? It is pretty
straightforward.
Plus, this is its second submission. It already includes the requested
changes from the first time (~1y ago).
Thanks in advance,
Manolis
On Sun, 4 Jul 2021 at 18:13, Manolis Stamatogiannakis
wrote:
> Allows shifting of subtitle display times to align them with the video.
> This avoids having to rewrite the subtitle file in order to display
> subtitles correctly when input is seeked (-ss).
> Also handy for minor subtitle timing corrections without rewriting the
> subtitles file.
>
> Signed-off-by: Manolis Stamatogiannakis
> ---
> doc/filters.texi | 11
> libavfilter/vf_subtitles.c | 55 +-
> 2 files changed, 59 insertions(+), 7 deletions(-)
>
> diff --git a/doc/filters.texi b/doc/filters.texi
> index 61c4cfc150..eebf455692 100644
> --- a/doc/filters.texi
> +++ b/doc/filters.texi
> @@ -19474,6 +19474,9 @@ Common @ref{subtitles}/@ref{ass} filter options:
> @item filename, f
> Set the filename of the subtitle file to read. It must be specified.
>
> +@item shift
> +Shift subtitles timings by the specified amount.
> +
> @item original_size
> Specify the size of the original video, the video for which the ASS file
> was composed. For the syntax of this option, check the
> @@ -19487,6 +19490,9 @@ These fonts will be used in addition to whatever
> the font provider uses.
>
> @item alpha
> Process alpha channel, by default alpha channel is untouched.
> +
> +@item shift
> +Shift subtitles timings by the specified amount.
> @end table
>
> Additional options for @ref{subtitles} filter:
> @@ -19533,6 +19539,11 @@ To make the subtitles stream from @file{sub.srt}
> appear in 80% transparent blue
> subtitles=sub.srt:force_style='Fontname=DejaVu
> Serif,PrimaryColour='
> @end example
>
> +To re-sync subtitles after seeking the input e.g. with @code{-ss 20:20},
> use:
> +@example
> +subtitles=filename=sub.srt:shift='-20\:20'
> +@end example
> +
> @section super2xsai
>
> Scale the input by 2x and smooth using the Super2xSaI (Scale and
> diff --git a/libavfilter/vf_subtitles.c b/libavfilter/vf_subtitles.c
> index ab32e1b7f3..2c7ce267e1 100644
> --- a/libavfilter/vf_subtitles.c
> +++ b/libavfilter/vf_subtitles.c
> @@ -52,6 +52,7 @@ typedef struct AssContext {
> char *filename;
> char *fontsdir;
> char *charenc;
> +int64_t shift;
> char *force_style;
> int stream_index;
> int alpha;
> @@ -66,11 +67,12 @@ typedef struct AssContext {
> #define FLAGS AV_OPT_FLAG_FILTERING_PARAM|AV_OPT_FLAG_VIDEO_PARAM
>
> #define COMMON_OPTIONS \
> -{"filename", "set the filename of file to read",
> OFFSET(filename), AV_OPT_TYPE_STRING, {.str = NULL}, 0, 0,
> FLAGS }, \
> -{"f", "set the filename of file to read",
> OFFSET(filename), AV_OPT_TYPE_STRING, {.str = NULL}, 0, 0,
> FLAGS }, \
> -{"original_size", "set the size of the original video (used to scale
> fonts)", OFFSET(original_w), AV_OPT_TYPE_IMAGE_SIZE, {.str = NULL}, 0, 0,
> FLAGS }, \
> -{"fontsdir", "set the directory containing the fonts to read",
> OFFSET(fontsdir), AV_OPT_TYPE_STRING, {.str = NULL}, 0, 0,
> FLAGS }, \
> -{"alpha", "enable processing of alpha channel",
> OFFSET(alpha), AV_OPT_TYPE_BOOL, {.i64 = 0 },
> 0,1, FLAGS }, \
> +{"filename", "set the filename of file to read",
> OFFSET(filename), AV_OPT_TYPE_STRING, {.str = NULL},
> 0, 0, FLAGS }, \
> +{"f", "set the filename of file to read",
> OFFSET(filename), AV_OPT_TYPE_STRING, {.str = NULL},
> 0, 0, FLAGS }, \
> +{"original_size", "set the size of the original video (used to scale
> fonts)", OFFSET(original_w), AV_OPT_TYPE_IMAGE_SIZE, {.str = NULL},
> 0, 0, FLAGS }, \
> +{"fontsdir", "set the directory containing the fonts to read",
> OFFSET(fontsdir), AV_OPT_TYPE_STRING, {.str = NULL},
> 0, 0, FLAGS }, \
> +{"alpha", "enable processing of alpha channel",
> OFFSET(alpha), AV_OPT_TYPE_BOOL, {.i64 = 0 },
> 0, 1, FLAGS }, \
> +{"shift", "shift subtitles timing",
> OFFSET(shift), AV_OPT_TYPE_DURATION, {.i64 = 0 },
> INT64_MIN, INT64_MAX, FLAGS }, \
>
> /* libass supports a log le
