[gstreamer-bugs] [Bug 403739] ffenc_mpeg1/2video segfaults

[GStreamer (bugzilla.gnome.org)]

Do not reply to this via email (we are currently unable to handle email
responses and they get discarded).  You can add comments to this bug at
http://bugzilla.gnome.org/show_bug.cgi?id=403739

  GStreamer | gst-ffmpeg | Ver: 0.10.2





------- Comment #4 from Mark Nauwelaerts  2007-03-02 15:29 UTC -------
Well, it's fixed, so this is mainly "for the record" ;-)

(In reply to comment #2)
> Looking deeper I can't see the relationship between setting proper
> intra_dc_precision and the fact that s->y_dc_scale_table has bogus value (here
> 0x1).

I am admittedly not particularly knowledgeable in lower levels of encoding,
but at least both dc coefficient (precision) and q scale affect quality;
and it is imaginable (?) that y__dc_scale__table (setup) is somehow affected
by the precision/bits of dc coefficient (in ffmpeg then at least apparently
;-))

(In reply to comment #3)
> Also, you say setting a default of 0 for intra_dc_precision is what mencoder
> does. According to the mencoder man page, the default is 8 (ffmpeg default
> value):
> 
>       dc
>               intra DC precision in bits (default: 8).  If you specify
> vcodec=mpeg2video this value can be 8, 9, 10 or 11.

>From avcodec.h:
    /**
     * precision of the intra dc coefficient - 8.
     * - encoding: set by user
     * - decoding: unused
     */
     int intra_dc_precision;

So, due to the - 8 shift, the user level default of 8 does correspond to
setting intra_dc_precision = 0 (ah, ffmpeg ..., and we now also know that
ffmpeg does not really set it to a default of 0 aka 8 (?), or it would not
segfault)

> I'm commiting only the first part of the patch, which states mpeg1video and
> mpeg2video are part of the mpeg family since that makes sense and gets rid of
> the segfault.
Yes, it will, because the first part is sort of a superset of the second part.
By including it in the MPEG-family, intra_dc_precision is then one of the many
parameters that gets a default (of 0 aka 8) in this case.

> If you can come with proof that the second part of the patch is really needed,
> please open a new bug with the affected encoders.
Either part of the patch by itself would fix it.
The second part is/was there for additional safety;
if ever one would feel like "playing" with the categories so that
intra_dc_precision does not get a safe default anymore, there would be segfault
(danger) again.  Hence, a hard-coded default = mencoder was provided.
As such, it is not needed for MPEGx in (the now) present setting, and have no
proof forthcoming for other encoders, but it is likely a safe(r) default given
mencoder's practice.


-- 
Configure bugmail: http://bugzilla.gnome.org/userprefs.cgi?tab=email