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

[Michael Niedermayer]

On Thu, Jul 03, 2014 at 06:50:21PM +0200, Oliver Fromme wrote:
> 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?

probably a good idea


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

split
applied

thanks

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

It is dangerous to be right in matters on which the established authorities
are wrong. -- Voltaire
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20140704/e1dac5c8/attachment.asc>