Re: [FFmpeg-devel] [PATCH 1/2] libavformat/mxfenc.c: Fix segfault when writing an audio packet when there has not been a video one.

[Alexis Ballier]

On Wed, 21 Oct 2015 22:43:04 +0200
Tomas Härdin  wrote:

> On Tue, 2015-10-20 at 16:43 +0200, Marton Balint wrote:
> > On Mon, 19 Oct 2015, Tomas Härdin wrote:
> >   
> > > On Mon, 2015-10-19 at 11:40 +0200, Alexis Ballier wrote:  
> > >> On Mon, 19 Oct 2015 10:30:00 +0200
> > >> Michael Niedermayer  wrote:
> > >>  
> > >>> On Fri, Oct 16, 2015 at 10:42:32AM +0200, Alexis Ballier
> > >>> wrote:  
> >  This happens when writing the trailer of a file containing
> >  audio but that has not muxed any video packet. Fixes ticket
> >  #4817.
> > 
> >    
> > >>>
> > >>> from IRC:
> > >>>  maybe it should print a warning that there has been no
> > >>> video?  
> > >>
> > >> maybe, but what would be the right condition ? just this case ?
> > >> or when writing any edit unit without video? or... ?
> > >> I haven't been able to find clear references on this (well,
> > >> those smpte specs I googled aren't esp. easy to read either),
> > >> but e.g. the muxer refuses to mux without exactly one video
> > >> stream, so maybe, an empty video stream is also an error; I
> > >> don't know  
> > >
> > > I'm not particularly fond of second-guessing these kinds of
> > > things. D-10 is really anal about what constitutes a legal file,
> > > so it's probably best not to write anything if something seems
> > > amiss. In the end, the real test for correctness is whether the
> > > output works with all gear for your particular use case. But
> > > segfaults are important to fix, so I'm not too concerned here.
> > > Plus my main concern is mxfdec, not mxfenc. 
> > 
> > Why not simply return error and print an error explaining "An audio
> > packet was muxed before a video, this is not supported."
> > 
> > Segfault -> error is much better than segfault -> possibly broken
> > file.  
> 
> Yes, simply bailing out is infinitely better than segfaulting. It's
> also the more conservative approach, which I am for

ok, so assuming the error condition is 'muxing first audio packet
before first video packet (if any)', I'll re-send with that warning
added
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 1/2] libavformat/mxfenc.c: Fix segfault when writing an audio packet when there has not been a video one.

[Tomas Härdin]

On Tue, 2015-10-20 at 16:43 +0200, Marton Balint wrote:
> On Mon, 19 Oct 2015, Tomas Härdin wrote:
> 
> > On Mon, 2015-10-19 at 11:40 +0200, Alexis Ballier wrote:
> >> On Mon, 19 Oct 2015 10:30:00 +0200
> >> Michael Niedermayer  wrote:
> >>
> >>> On Fri, Oct 16, 2015 at 10:42:32AM +0200, Alexis Ballier wrote:
>  This happens when writing the trailer of a file containing audio
>  but that has not muxed any video packet. Fixes ticket #4817.
> 
>  
> >>>
> >>> from IRC:
> >>>  maybe it should print a warning that there has been no
> >>> video?
> >>
> >> maybe, but what would be the right condition ? just this case ? or when
> >> writing any edit unit without video? or... ?
> >> I haven't been able to find clear references on this (well, those smpte
> >> specs I googled aren't esp. easy to read either), but e.g. the muxer
> >> refuses to mux without exactly one video stream, so maybe, an empty
> >> video stream is also an error; I don't know
> >
> > I'm not particularly fond of second-guessing these kinds of things. D-10
> > is really anal about what constitutes a legal file, so it's probably
> > best not to write anything if something seems amiss. In the end, the
> > real test for correctness is whether the output works with all gear for
> > your particular use case. But segfaults are important to fix, so I'm not
> > too concerned here. Plus my main concern is mxfdec, not mxfenc.
> >
> 
> Why not simply return error and print an error explaining "An audio packet 
> was muxed before a video, this is not supported."
> 
> Segfault -> error is much better than segfault -> possibly broken file.

Yes, simply bailing out is infinitely better than segfaulting. It's also
the more conservative approach, which I am for

/Tomas


signature.asc
Description: This is a digitally signed message part
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 1/2] libavformat/mxfenc.c: Fix segfault when writing an audio packet when there has not been a video one.

[Marton Balint]

On Mon, 19 Oct 2015, Tomas Härdin wrote:


On Mon, 2015-10-19 at 11:40 +0200, Alexis Ballier wrote:

On Mon, 19 Oct 2015 10:30:00 +0200
Michael Niedermayer  wrote:


On Fri, Oct 16, 2015 at 10:42:32AM +0200, Alexis Ballier wrote:

This happens when writing the trailer of a file containing audio
but that has not muxed any video packet. Fixes ticket #4817.




from IRC:
 maybe it should print a warning that there has been no
video?


maybe, but what would be the right condition ? just this case ? or when
writing any edit unit without video? or... ?
I haven't been able to find clear references on this (well, those smpte
specs I googled aren't esp. easy to read either), but e.g. the muxer
refuses to mux without exactly one video stream, so maybe, an empty
video stream is also an error; I don't know


I'm not particularly fond of second-guessing these kinds of things. D-10
is really anal about what constitutes a legal file, so it's probably
best not to write anything if something seems amiss. In the end, the
real test for correctness is whether the output works with all gear for
your particular use case. But segfaults are important to fix, so I'm not
too concerned here. Plus my main concern is mxfdec, not mxfenc.



Why not simply return error and print an error explaining "An audio packet 
was muxed before a video, this is not supported."


Segfault -> error is much better than segfault -> possibly broken file.

Regards,
Marton
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 1/2] libavformat/mxfenc.c: Fix segfault when writing an audio packet when there has not been a video one.

[Michael Niedermayer]

On Fri, Oct 16, 2015 at 10:42:32AM +0200, Alexis Ballier wrote:
> This happens when writing the trailer of a file containing audio but that has 
> not muxed any video packet.
> Fixes ticket #4817.
> 
> 

from IRC:
 maybe it should print a warning that there has been no video?

[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

It is what and why we do it that matters, not just one of them.


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 1/2] libavformat/mxfenc.c: Fix segfault when writing an audio packet when there has not been a video one.

[Carl Eugen Hoyos]

Alexis Ballier  gentoo.org> writes:

> This happens when writing the trailer of a file containing 
> audio but that has not muxed any video packet.
> Fixes ticket #4817.

Please also mention #4914 if appropriate.

Merci, Carl Eugen

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 1/2] libavformat/mxfenc.c: Fix segfault when writing an audio packet when there has not been a video one.

[Alexis Ballier]

On Mon, 19 Oct 2015 10:30:00 +0200
Michael Niedermayer  wrote:

> On Fri, Oct 16, 2015 at 10:42:32AM +0200, Alexis Ballier wrote:
> > This happens when writing the trailer of a file containing audio
> > but that has not muxed any video packet. Fixes ticket #4817.
> > 
> >   
> 
> from IRC:
>  maybe it should print a warning that there has been no
> video?

maybe, but what would be the right condition ? just this case ? or when
writing any edit unit without video? or... ?
I haven't been able to find clear references on this (well, those smpte
specs I googled aren't esp. easy to read either), but e.g. the muxer
refuses to mux without exactly one video stream, so maybe, an empty
video stream is also an error; I don't know
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 1/2] libavformat/mxfenc.c: Fix segfault when writing an audio packet when there has not been a video one.

[Tomas Härdin]

On Mon, 2015-10-19 at 11:40 +0200, Alexis Ballier wrote:
> On Mon, 19 Oct 2015 10:30:00 +0200
> Michael Niedermayer  wrote:
> 
> > On Fri, Oct 16, 2015 at 10:42:32AM +0200, Alexis Ballier wrote:
> > > This happens when writing the trailer of a file containing audio
> > > but that has not muxed any video packet. Fixes ticket #4817.
> > > 
> > >   
> > 
> > from IRC:
> >  maybe it should print a warning that there has been no
> > video?
> 
> maybe, but what would be the right condition ? just this case ? or when
> writing any edit unit without video? or... ?
> I haven't been able to find clear references on this (well, those smpte
> specs I googled aren't esp. easy to read either), but e.g. the muxer
> refuses to mux without exactly one video stream, so maybe, an empty
> video stream is also an error; I don't know

I'm not particularly fond of second-guessing these kinds of things. D-10
is really anal about what constitutes a legal file, so it's probably
best not to write anything if something seems amiss. In the end, the
real test for correctness is whether the output works with all gear for
your particular use case. But segfaults are important to fix, so I'm not
too concerned here. Plus my main concern is mxfdec, not mxfenc.

/Tomas


signature.asc
Description: This is a digitally signed message part
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH 1/2] libavformat/mxfenc.c: Fix segfault when writing an audio packet when there has not been a video one.

[Alexis Ballier]

This happens when writing the trailer of a file containing audio but that has 
not muxed any video packet.
Fixes ticket #4817.



This ticket also highlights the fact that mpeg2 video encoder produces no 
output when it has received less frames than its delay.
---
 libavformat/mxfenc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavformat/mxfenc.c b/libavformat/mxfenc.c
index 84ce979..f9b9516 100644
--- a/libavformat/mxfenc.c
+++ b/libavformat/mxfenc.c
@@ -2451,7 +2451,7 @@ static int mxf_write_packet(AVFormatContext *s, AVPacket 
*pkt)
 mxf->body_offset += KAG_SIZE; // size of system element
 }
 mxf->edit_units_count++;
-} else if (!mxf->edit_unit_byte_count && st->index == 1) {
+} else if (!mxf->edit_unit_byte_count && mxf->edit_units_count && 
st->index == 1) {
 mxf->index_entries[mxf->edit_units_count-1].slice_offset =
 mxf->body_offset - 
mxf->index_entries[mxf->edit_units_count-1].offset;
 }
-- 
2.6.1

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel