On Thu, Jul 16, 2015 at 07:19:40PM +0200, wm4 wrote: > On Thu, 16 Jul 2015 15:33:31 +0200 > Michael Niedermayer <mich...@niedermayer.cc> wrote: > > > On Thu, Jul 16, 2015 at 12:54:24PM +0200, wm4 wrote: > > [...] > > > > diff --git a/libavcodec/webp.c b/libavcodec/webp.c > > > > index 723a847..8caa6a2 100644 > > > > --- a/libavcodec/webp.c > > > > +++ b/libavcodec/webp.c > > > > @@ -1417,6 +1417,7 @@ static int webp_decode_frame(AVCodecContext > > > > *avctx, void *data, int *got_frame, > > > > chunk_size, 0); > > > > if (ret < 0) > > > > return ret; > > > > + avctx->properties |= FF_CODEC_PROPERTY_LOSSLESS; > > > > } > > > > bytestream2_skip(&gb, chunk_size); > > > > break; > > > > > > The lossless mode can apparently change mid-stream, so it should NOT be > > > an AVCodecContext field. Use per-frame side-data instead. > > > > "everything" can change mid stream, this field is not about if a frame > > Which is why the current bloated AVCodecContext approach is wrong. > > > is lossless or even if the whole file is guranteed to be lossless > > (for such gurantee the whole file would need to be parsed which is > > not practical for gathering some secondary bit of information about a > > stream) > > > > This field is simply to indicate if a stream appears to be lossless > > based on parsing headers or the first keyframe. > > So it is a property of the stream or decoder instance, similar to > > the width, height, format, sample rate, bit rate, ... fields in > > AVCodecContext are properties of a stream or codec instance even > > though they too could and do change per frame. > > So it exists for ffprobe? If ffprobe (or similar things) want to gather > information based on the first frame, they should just decode the first > frame, instead of requiring tons of fragile magic in libavformat.
ffprobe, yes but also anything that chooses a stream out of several may be interrested if a stream is likely lossless or not. and that potentially could be interresting for most tools using the libs. That choosing also can be automatic or manual both need some knowledge over a stream to choose which to use if there are several and yes all these tools could decode the first frame (and 2nd if the first isnt a keyframe, and 3rd, ... until width/height/... is known but i think its better if libavformat is able to do that and it doesnt need to be duplicated in each tool which is interrested in such information. > > In any case, adding even more rarely set fields to an already big > struct doesn't sound like a very good idea to me. With side-data at > least there's a certain discoverability and orderliness. It gets > obscure stuff out of the way. i dont mind if side data or meta data is used if it is possible and people prefer it ATM i think its not possible as theres no side data in the structs needed. also the reason why iam not a big fan of side data is that its a quite heavy system, needing malloc(), needing to deal with malloc failures, needing to iterate over lists of structs to find out if a specific "key" is set. It feels to me like total overkill to use all this for basically setting a single bit flag for a stream/codec. But like i said, i dont mind it if thats what people want to use for this and the people who want it also do the work [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB The bravest are surely those who have the clearest vision of what is before them, glory and danger alike, and yet notwithstanding go out to meet it. -- Thucydides
signature.asc
Description: Digital signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
