diff mbox series

[FFmpeg-devel] lavfi/subtitles: Fix subtitles default for ass option shaping

Message ID CAB0OVGp=Xi=X2x139VnvLyE6fZLkj2YQRWXGEMMYV+Yfe3uA2w@mail.gmail.com
State New
Headers show
Series [FFmpeg-devel] lavfi/subtitles: Fix subtitles default for ass option shaping | expand

Checks

Context Check Description
andriy/default pending
andriy/make success Make finished
andriy/make_fate success Make fate finished

Commit Message

Carl Eugen Hoyos June 20, 2020, 5:27 p.m. UTC 
Hi!

Attached patch fixes ticket #8738 (and #3165) for me.

Please comment, Carl Eugen
Subject: [PATCH] lavfi/subtitles: Fix the default value for the ass option
 shaping.

Fixes ticket #8738
---
 libavfilter/vf_subtitles.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Paul B Mahol June 21, 2020, 10:04 a.m. UTC | #1 
NAK.

On 6/20/20, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote:
> Hi!
>
> Attached patch fixes ticket #8738 (and #3165) for me.
>
> Please comment, Carl Eugen
>
Carl Eugen Hoyos June 21, 2020, 11:15 a.m. UTC | #2 
Am So., 21. Juni 2020 um 12:04 Uhr schrieb Paul B Mahol <onemda@gmail.com>:
>
> NAK.

Would you like to elaborate?

Carl Eugen
Paul B Mahol June 21, 2020, 11:54 a.m. UTC | #3 
On 6/21/20, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote:
> Am So., 21. Juni 2020 um 12:04 Uhr schrieb Paul B Mahol <onemda@gmail.com>:
>>
>> NAK.
>
> Would you like to elaborate?

Need clear explanation why. Not just what is done.

>
> Carl Eugen
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Hendrik Leppkes June 21, 2020, 1:44 p.m. UTC | #4 
On Sun, Jun 21, 2020 at 1:41 PM Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote:
>
> Am So., 21. Juni 2020 um 12:04 Uhr schrieb Paul B Mahol <onemda@gmail.com>:
> >
> > NAK.
>
> Would you like to elaborate?
>

It does seem a bit weird to set the default like this. Shouldn't the
default come from the options table, which seems to default to -1? And
if that doesn't work - why not?

- Hendrik
Carl Eugen Hoyos June 21, 2020, 4:05 p.m. UTC | #5 
Am So., 21. Juni 2020 um 15:52 Uhr schrieb Hendrik Leppkes
<h.leppkes@gmail.com>:
>
> On Sun, Jun 21, 2020 at 1:41 PM Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote:
> >
> > Am So., 21. Juni 2020 um 12:04 Uhr schrieb Paul B Mahol <onemda@gmail.com>:
> > >
> > > NAK.
> >
> > Would you like to elaborate?
>
> It does seem a bit weird to set the default like this. Shouldn't the
> default come from the options table, which seems to default to -1? And
> if that doesn't work - why not?

As I tried to explain in the commit message, the option does not
exist for the subtitles filter.

Carl Eugen
Hendrik Leppkes June 21, 2020, 4:27 p.m. UTC | #6 
On Sun, Jun 21, 2020 at 6:05 PM Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote:
>
> Am So., 21. Juni 2020 um 15:52 Uhr schrieb Hendrik Leppkes
> <h.leppkes@gmail.com>:
> >
> > On Sun, Jun 21, 2020 at 1:41 PM Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote:
> > >
> > > Am So., 21. Juni 2020 um 12:04 Uhr schrieb Paul B Mahol <onemda@gmail.com>:
> > > >
> > > > NAK.
> > >
> > > Would you like to elaborate?
> >
> > It does seem a bit weird to set the default like this. Shouldn't the
> > default come from the options table, which seems to default to -1? And
> > if that doesn't work - why not?
>
> As I tried to explain in the commit message, the option does not
> exist for the subtitles filter.
>

The commit message does not really explain. If you add that, I reckon its fine.

- Hendrik
diff mbox series

Patch

diff --git a/libavfilter/vf_subtitles.c b/libavfilter/vf_subtitles.c
index 1bd42391e0..5a4b642be7 100644
--- a/libavfilter/vf_subtitles.c
+++ b/libavfilter/vf_subtitles.c
@@ -307,6 +307,7 @@  static av_cold int init_subtitles(AVFilterContext *ctx)
     AVStream *st;
     AVPacket pkt;
     AssContext *ass = ctx->priv;
+    ass->shaping = -1;
 
     /* Init libass */
     ret = init(ctx);