[FFmpeg-devel] [PATCH] Fix off-by-one error in dvdsubdec.c

[Oliver Fromme]

Carl Eugen Hoyos wrote:
 > On Thursday 03 July 2014 04:12:12 pm Oliver Fromme wrote:
 > > I recently noticed that the lowest pixel rows from VOBSUB
 > > subtitles are missing.
 > 
 > Thank you for looking into this!
 > Iirc, the issue is (also) reproducible with 
 > http://samples.ffmpeg.org/MPEG-VOB/ClosedCaptions/Starship_Troopers.vob

Yes, I looked a quite a lot of VOB files from various DVDs,
and the issue occured with *all* of their subtitles.

By the way, there's another problem that's similar, but not
related to this fix:  Many (hardware) players omit the lowest
row of pixels if the number of rows in the subtitle is odd.
I'm not sure if the DVD standard requires that it must be even
(I believe it doesn't), or if this is a bug in the firmware of
common players.  Affected players are, for example, the Asus
"O!Play" and the Mede8er X600 (VOBSUB in MKV only; the problem
is not present when playing ISO files).

As a work-around, I would like to modify dvdsubenc.c to add
an empty (i.e. transparent) row to a subtitle if its height
is odd, in order to make it even.  In fact I already have a
patch for this that works; it's trivial.  But I'm not sure
if that patch would be accepted, because it is only a work-
around for a bug in some devices, and it adds a tiny overhead
(on average, it adds one byte per subtitle).  Well, maybe I
should disable the work-around by default and add an encoder
flag that enables it.  What do you think?  Would such a patch
be accepted for inclusion in ffmpeg?

 > > Also
 > 
 > The maintainer may disagree, but generally "also" in the commit message 
 > indicates that you should send two patches.
 > (And this does not look like an exception, on the contrary.)

Good point.  You are right, it makes sense to split this in
two patches.  I didn't thought of doing that, because -- in my
mind -- the debugging aid and the actual bug fix belonged
together (I wouldn't have done one without the other).  But of
course you are right, they work independent from each other.

Should I resend the patches separately, or can the person who
is going to commit this do the splitting?  It should be easy,
because the first patch (the actual bug fix) changes only one
line of source code.

I'm sorry for the inconvenience.

Best regards
   Oliver


-- 
``We are all but compressed light'' (Albert Einstein)