Re: [FFmpeg-devel] [PATCH] avformat/srtenc: avoid segmentation fault when transcoding from SSA to SRT
On Sun, Mar 08, 2015 at 10:57:54AM +0100, Nicolas George wrote: L'octidi 18 ventôse, an CCXXIII, Clement Boesch a écrit : Try adding tags with no text maybe. You may also try ASS drawing mode, but FFmpeg probably doesn't do cray stuff about it. I tried various combinations, including empty styles, drawing mode, Picture, Comment, empty lines, missing comma, nothing produces a crash. Ok, I will remove the patch on my side, and chime in if I get the segfault again. Thanks for investigating, sorry for the noise. Regards. -- Gilles. pgpfq8h5xrkbH.pgp Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avformat/srtenc: avoid segmentation fault when transcoding from SSA to SRT
L'octidi 18 ventôse, an CCXXIII, Gilles Chanteperdrix a écrit : If I read ffmpeg current documentation here: http://www.ffmpeg.org/doxygen/2.5/group__lavc__core.html#ga828218dcb8874ab2c5af8d486c365421 It seems that when a subtitle rect type is SUBTITLE_ASS (which is the case in the code which my patch modified), there is no guarantee that the text field contains something. You read the documentation correctly, but the text field you are testing does not belong to AVSubtitleRect but to ASSDialog. And you can see that a few lines above, the code checks that the subtitles are actually SUBTITLE_ASS. Is not there some way in the ssa format to declare some graphic elements which are not text? Theoretically yes, but the ASS demuxer just ignores them. In my modest experience, bugs do not just disappear if you do not fix them. In my modest experience, it can happen when the code that contains them is reworked in depth, or when it was a consequence of another bug that was fixed. This code has been reworked in depth. Maybe it is not a perfect patch, but if subtitles can contain graphic elements mixed with text, it seems better to translate the text only when the target codec only supports text, than to fail with a segmentation fault. I do not deny that. But until proven otherwise, there is no segmentation fault, the code as is works very well. Regards, -- Nicolas George signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avformat/srtenc: avoid segmentation fault when transcoding from SSA to SRT
L'octidi 18 ventôse, an CCXXIII, Clement Boesch a écrit : Try adding tags with no text maybe. You may also try ASS drawing mode, but FFmpeg probably doesn't do cray stuff about it. I tried various combinations, including empty styles, drawing mode, Picture, Comment, empty lines, missing comma, nothing produces a crash. Regards, -- Nicolas George signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avformat/srtenc: avoid segmentation fault when transcoding from SSA to SRT
On Sun, Mar 08, 2015 at 12:27:49AM +0100, Gilles Chanteperdrix wrote: On Sat, Mar 07, 2015 at 11:46:44PM +0100, Nicolas George wrote: Le septidi 17 ventôse, an CCXXIII, Gilles Chanteperdrix a écrit : You need an ASS file that produces a sub-title frame without text. I have been carrying this patch since 2013, I am afraid I no longer have the problematic file. I tried an ASS file with an empty text part, and it did not crash either. Is not there some way in the ssa format to declare some graphic elements which are not text? Try adding tags with no text maybe. You may also try ASS drawing mode, but FFmpeg probably doesn't do cray stuff about it. -- Clément B. pgpwcLFtl4Ba6.pgp Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avformat/srtenc: avoid segmentation fault when transcoding from SSA to SRT
Le septidi 17 ventôse, an CCXXIII, Gilles Chanteperdrix a écrit : When transcoding to SRT, do not assume that each sub-title frame contains text. Can you explain how to reproduce the segfault? I tried transcoding an ASS file and it worked. Regards, -- Nicolas George signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avformat/srtenc: avoid segmentation fault when transcoding from SSA to SRT
On Sat, Mar 07, 2015 at 11:46:44PM +0100, Nicolas George wrote: Le septidi 17 ventôse, an CCXXIII, Gilles Chanteperdrix a écrit : You need an ASS file that produces a sub-title frame without text. I have been carrying this patch since 2013, I am afraid I no longer have the problematic file. I tried an ASS file with an empty text part, and it did not crash either. Is not there some way in the ssa format to declare some graphic elements which are not text? It is entirely possible the problem has disappeared due to some of the reworks of the text subtitles code since you experienced it. In my modest experience, bugs do not just disappear if you do not fix them. I am sorry to insist, but I am always suspicious of changes that go if the value is not what we expect, silently ignore it, because they frequently should return an error or hide a bug earlier in the code; Maybe it is not a perfect patch, but if subtitles can contain graphic elements mixed with text, it seems better to translate the text only when the target codec only supports text, than to fail with a segmentation fault. for these reasons, I would like either a more detailed analysis or a chance to see for myself before this patch is applied. Just to make things clear: you will not get such an analysis for me. I am perfectly fine with the patch applied in my local copy of ffmpeg. -- Gilles. pgpgGnMK76Xzl.pgp Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] avformat/srtenc: avoid segmentation fault when transcoding from SSA to SRT
When transcoding to SRT, do not assume that each sub-title frame contains
text.
Signed-off-by: Gilles Chanteperdrix gilles.chanteperd...@xenomai.org
---
libavcodec/srtenc.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/libavcodec/srtenc.c b/libavcodec/srtenc.c
index 3287970..ab790f8 100644
--- a/libavcodec/srtenc.c
+++ b/libavcodec/srtenc.c
@@ -241,6 +241,9 @@ static int srt_encode_frame(AVCodecContext *avctx,
for (; dialog num--; dialog++) {
s-alignment_applied = 0;
srt_style_apply(s, dialog-style);
+if (!dialog-text)
+continue;
+
ff_ass_split_override_codes(srt_callbacks, s, dialog-text);
}
}
--
1.8.4
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avformat/srtenc: avoid segmentation fault when transcoding from SSA to SRT
On Sat, Mar 07, 2015 at 11:46:44PM +0100, Nicolas George wrote: Le septidi 17 ventôse, an CCXXIII, Gilles Chanteperdrix a écrit : You need an ASS file that produces a sub-title frame without text. I have been carrying this patch since 2013, I am afraid I no longer have the problematic file. I tried an ASS file with an empty text part, and it did not crash either. It is entirely possible the problem has disappeared due to some of the reworks of the text subtitles code since you experienced it. I am sorry to insist, but I am always suspicious of changes that go if the value is not what we expect, silently ignore it, because they frequently should return an error or hide a bug earlier in the code; for these reasons, I would like either a more detailed analysis or a chance to see for myself before this patch is applied. If I read ffmpeg current documentation here: http://www.ffmpeg.org/doxygen/2.5/group__lavc__core.html#ga828218dcb8874ab2c5af8d486c365421 It seems that when a subtitle rect type is SUBTITLE_ASS (which is the case in the code which my patch modified), there is no guarantee that the text field contains something. -- Gilles. pgpoVl06GLm91.pgp Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avformat/srtenc: avoid segmentation fault when transcoding from SSA to SRT
On Sat, Mar 07, 2015 at 09:18:41PM +0100, Nicolas George wrote: Le septidi 17 ventôse, an CCXXIII, Gilles Chanteperdrix a écrit : When transcoding to SRT, do not assume that each sub-title frame contains text. Can you explain how to reproduce the segfault? I tried transcoding an ASS file and it worked. You need an ASS file that produces a sub-title frame without text. I have been carrying this patch since 2013, I am afraid I no longer have the problematic file. -- Gilles. pgprvrq1x6I6o.pgp Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avformat/srtenc: avoid segmentation fault when transcoding from SSA to SRT
Le septidi 17 ventôse, an CCXXIII, Gilles Chanteperdrix a écrit : You need an ASS file that produces a sub-title frame without text. I have been carrying this patch since 2013, I am afraid I no longer have the problematic file. I tried an ASS file with an empty text part, and it did not crash either. It is entirely possible the problem has disappeared due to some of the reworks of the text subtitles code since you experienced it. I am sorry to insist, but I am always suspicious of changes that go if the value is not what we expect, silently ignore it, because they frequently should return an error or hide a bug earlier in the code; for these reasons, I would like either a more detailed analysis or a chance to see for myself before this patch is applied. Regards, -- Nicolas George signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
