Re: [FFmpeg-devel] EOF and IO error checking

2019-08-23 Thread Michael Niedermayer
On Thu, Aug 22, 2019 at 10:47:17AM +0200, Marton Balint wrote:
> 
> 
> On Sun, 18 Aug 2019, Michael Niedermayer wrote:
> 
> >On Sat, Aug 17, 2019 at 09:10:59PM +0200, Marton Balint wrote:
> >>Hi,
> >>
> >>As you might now avio_feof() returns true both in case of actual EOF and in
> >>case of IO errors.
> >>
> >>Some demuxers (matroska) have special handling for this exact reason, e.g.:
> >>
> >>if (avio_feof(pb)) {
> >>if (pb->error) {
> >>return pb->error;
> >>} else {
> >>return AVERROR_EOF;
> >>}
> >>}
> >>
> >>Most of the demuxers do not, so there is a real chance that IO errrors are
> >>mistaken for EOF.
> >>
> >>What should we do about this?
> >>
> >>a) Fix every demuxer to return IO error if there is one.
> >>
> >>b) Add special code to ff_read_packet which checks if there is an error in
> >>the IO context and return that if there is?
> >
> >>
> >>c) Add code to ffmpeg.c which checks the IO context error code after every
> >>av_read_frame call?
> >
> >This while generally correct is not guranteed to be correct.
> >The internal IO context may have an IO error without the demuxer having an
> >error. From the possibility of reconnects to redundancy to errors after
> >all essential parts of a file ...
> >so this may need some flag per demuxer that either declares this relation
> >true or a flag declaring it false
> 
> Do you have an example in mind for a demuxer which specifically needs this?
> From the top of my head I can't think of one.
> 
> Reconnects happen in the protocol handlers, so errors there should not
> affect the AVIO context errors.
> 
> I am also sceptical about the use case of gracefully handling IO errors in
> non-essential parts of the file. Signalling IO errors is a lot more
> important and I bet there are cases where some demuxer won't set the packet
> corrupt flag (which is by the way not propagated correctly through the API)
> if it encounters an IO error when reading something.
> 
> In any case, I don't mind too much option a), if that what seems more clean
> but we will probably miss a few cases of the issue.

Some formats have an index at the end, which is optional and the file works 
fine without but if its missing or on a bad sector maybe the IO context would be
left in a state of error until the next packet is read.
Also some demuxers may have some form of buffer between what they read and
the output for the user, an IO error may be set in the IO context before
prior packets are returned.
we also read a few packets ahead to obtain resolution and such in some cases
again what the user gets as packet is from a buffer while the IO context
represents the future.
so simply using the IO contexts error state might not work unconditionally
when it works sure iam fine with it ...


Thanks

[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Its not that you shouldnt use gotos but rather that you should write
readable code and code with gotos often but not always is less readable


signature.asc
Description: PGP signature
___
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] EOF and IO error checking

2019-08-22 Thread James Almer
On 8/17/2019 4:10 PM, Marton Balint wrote:
> Hi,
> 
> As you might now avio_feof() returns true both in case of actual EOF and
> in case of IO errors.
> 
> Some demuxers (matroska) have special handling for this exact reason, e.g.:
> 
> if (avio_feof(pb)) {
>     if (pb->error) {
>     return pb->error;
>     } else {
>     return AVERROR_EOF;
>     }
> }
> 
> Most of the demuxers do not, so there is a real chance that IO errrors
> are mistaken for EOF.
> 
> What should we do about this?
> 
> a) Fix every demuxer to return IO error if there is one.
> 
> b) Add special code to ff_read_packet which checks if there is an error
> in the IO context and return that if there is?

Handling things in generic code is preferred over adding checks to every
demuxer, IMO.

Also, fixes in libavformat are always preferred over fixes/workarounds
in ffmpeg.c, since the latter will not have any effect on every other
library user.

> 
> c) Add code to ffmpeg.c which checks the IO context error code after
> every av_read_frame call?
> 
> Some other idea? Which one is preferred from these?
> 
> Thanks,
> Marton
> 
> 
> ___
> 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] EOF and IO error checking

2019-08-22 Thread Marton Balint



On Sun, 18 Aug 2019, Michael Niedermayer wrote:


On Sat, Aug 17, 2019 at 09:10:59PM +0200, Marton Balint wrote:

Hi,

As you might now avio_feof() returns true both in case of actual EOF and in
case of IO errors.

Some demuxers (matroska) have special handling for this exact reason, e.g.:

if (avio_feof(pb)) {
if (pb->error) {
return pb->error;
} else {
return AVERROR_EOF;
}
}

Most of the demuxers do not, so there is a real chance that IO errrors are
mistaken for EOF.

What should we do about this?

a) Fix every demuxer to return IO error if there is one.

b) Add special code to ff_read_packet which checks if there is an error in
the IO context and return that if there is?




c) Add code to ffmpeg.c which checks the IO context error code after every
av_read_frame call?


This while generally correct is not guranteed to be correct.
The internal IO context may have an IO error without the demuxer having an
error. From the possibility of reconnects to redundancy to errors after
all essential parts of a file ...
so this may need some flag per demuxer that either declares this relation
true or a flag declaring it false


Do you have an example in mind for a demuxer which specifically needs 
this? From the top of my head I can't think of one.


Reconnects happen in the protocol handlers, so errors there should not 
affect the AVIO context errors.


I am also sceptical about the use case of gracefully handling IO errors 
in non-essential parts of the file. Signalling IO errors is a lot more 
important and I bet there are cases where some demuxer won't set the 
packet corrupt flag (which is by the way not propagated correctly through 
the API) if it encounters an IO error when reading something.


In any case, I don't mind too much option a), if that what seems more 
clean but we will probably miss a few cases of the issue.


Thanks,
Marton
___
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] EOF and IO error checking

2019-08-18 Thread Michael Niedermayer
On Sat, Aug 17, 2019 at 09:10:59PM +0200, Marton Balint wrote:
> Hi,
> 
> As you might now avio_feof() returns true both in case of actual EOF and in
> case of IO errors.
> 
> Some demuxers (matroska) have special handling for this exact reason, e.g.:
> 
> if (avio_feof(pb)) {
> if (pb->error) {
> return pb->error;
> } else {
> return AVERROR_EOF;
> }
> }
> 
> Most of the demuxers do not, so there is a real chance that IO errrors are
> mistaken for EOF.
> 
> What should we do about this?
> 
> a) Fix every demuxer to return IO error if there is one.
> 
> b) Add special code to ff_read_packet which checks if there is an error in
> the IO context and return that if there is?

> 
> c) Add code to ffmpeg.c which checks the IO context error code after every
> av_read_frame call?

This while generally correct is not guranteed to be correct.
The internal IO context may have an IO error without the demuxer having an
error. From the possibility of reconnects to redundancy to errors after
all essential parts of a file ...
so this may need some flag per demuxer that either declares this relation
true or a flag declaring it false


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

Those who are best at talking, realize last or never when they are wrong.


signature.asc
Description: PGP signature
___
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] EOF and IO error checking

2019-08-17 Thread Marton Balint

Hi,

As you might now avio_feof() returns true both in case of actual EOF and 
in case of IO errors.


Some demuxers (matroska) have special handling for this exact reason, 
e.g.:


if (avio_feof(pb)) {
if (pb->error) {
return pb->error;
} else {
return AVERROR_EOF;
}
}

Most of the demuxers do not, so there is a real chance that IO errrors 
are mistaken for EOF.


What should we do about this?

a) Fix every demuxer to return IO error if there is one.

b) Add special code to ff_read_packet which checks if there is an error in 
the IO context and return that if there is?


c) Add code to ffmpeg.c which checks the IO context error code after every 
av_read_frame call?


Some other idea? Which one is preferred from these?

Thanks,
Marton


___
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".