[FFmpeg-devel] [PATCH]ffplay 2/2: Fix subtitle rendering for special cases

[Björn Axelsson]

>On Sun, Feb 01, 2009 at 11:19:37PM +0100, Bj?rn Axelsson wrote:
>> Second subtitle-related patch.
>>
>> The ffplay.c subtitle renderer (blend_subrect) fails to handle some of
>> the
>> special cases with odd widths, odd heights and/or odd positions. For
>> example it could insert an empty line, wraparound the subrect one pixel
>> vertically, misalign the UV components and generally make a mess of
>> things.
>>
>> This fix could have been smaller, since I didn't care to track down the
>> original error in the offset calculations. Instead I replaced the
>> calculations with an arguably simpler and seemingly more correct
>> implementation.
>
>great, iam in favor of someone rewriting blend_subrect()
>first see "C?dric Schieli [FFmpeg-devel] [RFC] Alpha support" there is
>some alpha blend code in there, with which the subtitle case should be
>merged evetually
>
>second the code should be < 1/2 as big as now
>
>your choice is between either
>A. writing a clean and minimal patch to fix the bug (and supply a clear
>   description of the bug
>B. provide a new (not diffed) implementation that is smaller, cleaner and
>   better (1/2 size and attached benchmarks are required)

or

C. Do nothing, since I do not want to spend my time debugging complex code
   that I have already replaced with less buggy and slightly less complex
   code and you do not want to review a non-minimal fix.

-- 
Bj?rn Axelsson
-------------- next part --------------
On Sun, Feb 01, 2009 at 11:19:37PM +0100, Bj?rn Axelsson wrote:
> Second subtitle-related patch.
> 
> The ffplay.c subtitle renderer (blend_subrect) fails to handle some of the
> special cases with odd widths, odd heights and/or odd positions. For
> example it could insert an empty line, wraparound the subrect one pixel
> vertically, misalign the UV components and generally make a mess of things.
> 
> This fix could have been smaller, since I didn't care to track down the
> original error in the offset calculations. Instead I replaced the
> calculations with an arguably simpler and seemingly more correct
> implementation.

great, iam in favor of someone rewriting blend_subrect()
first see "C?dric Schieli [FFmpeg-devel] [RFC] Alpha support" there is some
alpha blend code in there, with which the subtitle case should be merged
evetually

second the code should be < 1/2 as big as now

your choice is between either
A. writing a clean and minimal patch to fix the bug (and supply a clear
   description of the bug
B. provide a new (not diffed) implementation that is smaller, cleaner and
   better (1/2 size and attached benchmarks are required)


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

In a rich man's house there is no place to spit but his face.
-- Diogenes of Sinope
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090202/05d71cf0/attachment.pgp>
-------------- next part --------------
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel at mplayerhq.hu
https://lists.mplayerhq.hu/mailman/listinfo/ffmpeg-devel