Re: [FFmpeg-devel] [PATCH 04/10] libavcodec/ccaption_dec: reap_screen after flipping on EOC

[Aman Gupta]

On Fri, Jan 8, 2016 at 2:50 AM, Anshul Maheshwari 
wrote:

>
>
> On Thu, Jan 7, 2016 at 6:14 AM, Aman Gupta  wrote:
>
>> Probably should have written a longer commit message here. The EOC
>> command stands for "end of caption" aka "display buffer". It's used with
>> POPON mode, where characters are written to an off-screen buffer and EOC
>> flips the buffers to display what has been written so far. Thus, it makes
>> sense to reap the screen *after* flipping the active screen, not *before*.
>>
>> The previous behavior was simply wrong, but masked by other bugs also
>> fixed in this patchset.
>>
>> Aman
>>
>> On Tue, Jan 5, 2016 at 11:41 PM, Aman Gupta  wrote:
>>
>>> From: Aman Gupta 
>>>
>>> ---
>>>  libavcodec/ccaption_dec.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/libavcodec/ccaption_dec.c b/libavcodec/ccaption_dec.c
>>> index 9f17e77..5d4c568 100644
>>> --- a/libavcodec/ccaption_dec.c
>>> +++ b/libavcodec/ccaption_dec.c
>>> @@ -399,9 +399,9 @@ static void handle_erase(CCaptionSubContext *ctx,
>>> int n_screen)
>>>
>>>  static void handle_eoc(CCaptionSubContext *ctx)
>>>  {
>>> -reap_screen(ctx);
>>>  ctx->active_screen = !ctx->active_screen;
>>>  ctx->cursor_column = 0;
>>> +reap_screen(ctx);
>>>  }
>>>
>>>  static void handle_delete_end_of_row(CCaptionSubContext *ctx, char hi,
>>> char lo)
>>> --
>>> 2.5.3
>>>
>>>
>> I dont see it as bug, this problem comes because of change in 02/10 patch.
>
> handle_eoc always called handle_edm, am I missing some patch?
>

I removed the call to EDM from inside EOC, in "libavcodec/ccaption_dec:
reap_screen is not necessary when clearing screen or buffer"

Always calling EDM does not make sense, because the caption stream will
already send EDM when it is required. I think the EDM from EOC behavior was
a work around because previously "erase non-displayed memory" was not
implemented. See commit: libavcodec/ccaption_dec: implement "erase non
displayed memory"


>
> how does it matter, since this code has to be exectued sequentially? reap
> before or after I see them  at same instant or at same command.
>

This matters because reap_screen will look at active_screen. EOC means
"display buffer", so we must first flip the active screen to display the
off-screen buffer, and then reap it. If you reap first, you are reading
from the incorrect buffer.


>
>
> -Anshul
>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 04/10] libavcodec/ccaption_dec: reap_screen after flipping on EOC

[Anshul Maheshwari]

On Thu, Jan 7, 2016 at 6:14 AM, Aman Gupta  wrote:

> Probably should have written a longer commit message here. The EOC command
> stands for "end of caption" aka "display buffer". It's used with POPON
> mode, where characters are written to an off-screen buffer and EOC flips
> the buffers to display what has been written so far. Thus, it makes sense
> to reap the screen *after* flipping the active screen, not *before*.
>
> The previous behavior was simply wrong, but masked by other bugs also
> fixed in this patchset.
>
> Aman
>
> On Tue, Jan 5, 2016 at 11:41 PM, Aman Gupta  wrote:
>
>> From: Aman Gupta 
>>
>> ---
>>  libavcodec/ccaption_dec.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/libavcodec/ccaption_dec.c b/libavcodec/ccaption_dec.c
>> index 9f17e77..5d4c568 100644
>> --- a/libavcodec/ccaption_dec.c
>> +++ b/libavcodec/ccaption_dec.c
>> @@ -399,9 +399,9 @@ static void handle_erase(CCaptionSubContext *ctx, int
>> n_screen)
>>
>>  static void handle_eoc(CCaptionSubContext *ctx)
>>  {
>> -reap_screen(ctx);
>>  ctx->active_screen = !ctx->active_screen;
>>  ctx->cursor_column = 0;
>> +reap_screen(ctx);
>>  }
>>
>>  static void handle_delete_end_of_row(CCaptionSubContext *ctx, char hi,
>> char lo)
>> --
>> 2.5.3
>>
>>
> I dont see it as bug, this problem comes because of change in 02/10 patch.

handle_eoc always called handle_edm, am I missing some patch?

how does it matter, since this code has to be exectued sequentially? reap
before or after I see them  at same instant or at same command.

-Anshul
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 04/10] libavcodec/ccaption_dec: reap_screen after flipping on EOC

[Aman Gupta]

Probably should have written a longer commit message here. The EOC command
stands for "end of caption" aka "display buffer". It's used with POPON
mode, where characters are written to an off-screen buffer and EOC flips
the buffers to display what has been written so far. Thus, it makes sense
to reap the screen *after* flipping the active screen, not *before*.

The previous behavior was simply wrong, but masked by other bugs also fixed
in this patchset.

Aman

On Tue, Jan 5, 2016 at 11:41 PM, Aman Gupta  wrote:

> From: Aman Gupta 
>
> ---
>  libavcodec/ccaption_dec.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/libavcodec/ccaption_dec.c b/libavcodec/ccaption_dec.c
> index 9f17e77..5d4c568 100644
> --- a/libavcodec/ccaption_dec.c
> +++ b/libavcodec/ccaption_dec.c
> @@ -399,9 +399,9 @@ static void handle_erase(CCaptionSubContext *ctx, int
> n_screen)
>
>  static void handle_eoc(CCaptionSubContext *ctx)
>  {
> -reap_screen(ctx);
>  ctx->active_screen = !ctx->active_screen;
>  ctx->cursor_column = 0;
> +reap_screen(ctx);
>  }
>
>  static void handle_delete_end_of_row(CCaptionSubContext *ctx, char hi,
> char lo)
> --
> 2.5.3
>
>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH 04/10] libavcodec/ccaption_dec: reap_screen after flipping on EOC

[Aman Gupta]

From: Aman Gupta 

---
 libavcodec/ccaption_dec.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavcodec/ccaption_dec.c b/libavcodec/ccaption_dec.c
index 9f17e77..5d4c568 100644
--- a/libavcodec/ccaption_dec.c
+++ b/libavcodec/ccaption_dec.c
@@ -399,9 +399,9 @@ static void handle_erase(CCaptionSubContext *ctx, int 
n_screen)
 
 static void handle_eoc(CCaptionSubContext *ctx)
 {
-reap_screen(ctx);
 ctx->active_screen = !ctx->active_screen;
 ctx->cursor_column = 0;
+reap_screen(ctx);
 }
 
 static void handle_delete_end_of_row(CCaptionSubContext *ctx, char hi, char lo)
-- 
2.5.3

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel