[FFmpeg-devel] img2dec: jpeg_probe logic
Hey, Trying to fix a bug #6113, I stumbled upon some strange logic in libavformat/img2dec.c:jpeg_probe. It accepts first 2048 bytes of jpeg stream, and tries to read it with some state machine. If it doesn't look like jpeg stream, it returns 0 ("it's definitely not a jpeg"). After it read through the buffer, it does this: if (state == EOI) return AVPROBE_SCORE_EXTENSION + 1; if (state == SOS) return AVPROBE_SCORE_EXTENSION / 2; return AVPROBE_SCORE_EXTENSION / 8; That doesn't make sense to me. All we read so far made sense for jpeg reader, it definitely looks like a jpeg image, but for some reason our confidence is AVPROBE_SCORE_EXTENSION _DIVIDED_ by 2 or 8 (in other words, "it would look more jpeg if it had .jpg extension). Compare it with png, for example: static int png_probe(AVProbeData *p) { const uint8_t *b = p->buf; if (AV_RB64(b) == 0x89504e470d0a1a0a) return AVPROBE_SCORE_MAX - 1; return 0; } "Return max confidence, if it has proper signature, 0 otherwise" Shouldn't it be something like this? if (state == EOI) return AVPROBE_SCORE_EXTENSION + 3; if (state == SOS) return AVPROBE_SCORE_EXTENSION + 2; return AVPROBE_SCORE_EXTENSION + 1; I'll send a patch if there're no objections. Thanks! V. Kalinsky ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] img2dec: jpeg_probe logic
Can we change mjpeg_probe() then? It has the same problem. "Everything was valid, and there's more than two frames, but it still has lower score than extension-based guess" libavformat/rawdec.c:184 if (nb_invalid == 0 && nb_frames > 2) return AVPROBE_SCORE_EXTENSION / 2; Vadim ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] bprint.h can't be included in C++ code
C++ does not support anonymous struct. 0001-C-compatible-AVBPrint-definition.patch Description: Binary data Vadim Kalinsky kalinsky.ru signature.asc Description: Message signed with OpenPGP using GPGMail ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] bprint.h can't be included in C++ code
> identifiers starting with a double > underscode are reserved for the implementation, and therefore can not be > used. Yep, my bad. > Apart from the fact that it makes the macro hackery vastly less readable > (maybe some indentation would help), Maybe I'm little contaminated with modern PLs, but I don't see huge difference in readability between the old and the new versions. Both are terrible :-) 0001-Squashed.patch Description: Binary data Vadim Kalinsky kalinsky.ru signature.asc Description: Message signed with OpenPGP using GPGMail ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] bprint.h can't be included in C++ code
> Nit: space after commas. > Is it necessary to typedef the structure on top of declaring it? In C, this: > {...} is legal, no need to typedef it. > Apart from that, I think the helper structure should be scoped in the > ff_namespace > Nit: no empty line between doxy and declaration. Patch attached. 0001-Squashed-commits.patch Description: Binary data > And I wonder: does Doxygen understand the resulting construct? I guess that (not good) behaviour is not changed. BTW, isn't the whole stuff slightly over-engineered? Why is "binary compatibility" needed (and with what?). Wouldn't it be easier in terms of compatibility, readability and doxygenability to define simple C structure? struct AVBPrint { char *str; /**< string so far */ unsigned len; /**< length so far */ unsigned size; /**< allocated memory */ unsigned size_max; /**< maximum allocated memory */ char reserved_internal_buffer[1024]; }; Vadim Kalinsky kalinsky.ru ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel