Re: [FFmpeg-devel] [PATCH] ffplay: log possible error of SDL_EnableKeyRepeat

2015-10-11 Thread Ganesh Ajjanagadde 
On Fri, Oct 9, 2015 at 9:03 AM, Ganesh Ajjanagadde  wrote:
> On Fri, Oct 9, 2015 at 4:21 AM, wm4  wrote:
>> On Thu,  8 Oct 2015 18:46:44 -0400
>> Ganesh Ajjanagadde  wrote:
>>
>>> Note that for the current SDL source code, 0 is always returned.
>>> Nevertheless, this makes the code more robust.
>>>
>>> Signed-off-by: Ganesh Ajjanagadde 
>>> ---
>>>  ffplay.c | 3 ++-
>>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/ffplay.c b/ffplay.c
>>> index 79f430d..c143e96 100644
>>> --- a/ffplay.c
>>> +++ b/ffplay.c
>>> @@ -3814,7 +3814,8 @@ int main(int argc, char **argv)
>>>  SDL_EventState(SDL_SYSWMEVENT, SDL_IGNORE);
>>>  SDL_EventState(SDL_USEREVENT, SDL_IGNORE);
>>>
>>> -SDL_EnableKeyRepeat(SDL_DEFAULT_REPEAT_DELAY, 
>>> SDL_DEFAULT_REPEAT_INTERVAL);
>>> +if (SDL_EnableKeyRepeat(SDL_DEFAULT_REPEAT_DELAY, 
>>> SDL_DEFAULT_REPEAT_INTERVAL) < 0)
>>> +av_log(NULL, AV_LOG_ERROR, "SDL_EnableKeyRepeat(): %s\n", 
>>> SDL_GetError());
>>>
>>>  if (av_lockmgr_register(lockmgr)) {
>>>  av_log(NULL, AV_LOG_FATAL, "Could not initialize lock manager!\n");
>>
>> How does this make the code more "robust"?
>
> If in future some limitations are placed on the repeat delay/repeat
> interval apart from nonnegativity, or if someone accidentally changes
> the repeat delay to e.g something dynamic and forgot to check that it
> is > 0, etc we would get a log message from ffplay allowing quick
> fixing of the issue.
>
> I don't know if "robust" is the right word for it, maybe "more complete"?

I drop this patch, on reflection I am anyway ambivalent to it - it
increases the code verbosity for minimal gain: in the event that the
delay values are changed, this code will anyway be examine; logging
for debugging purposes is not really that helpful.

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

Re: [FFmpeg-devel] [PATCH] ffplay: log possible error of SDL_EnableKeyRepeat

2015-10-09 Thread Ganesh Ajjanagadde 
On Fri, Oct 9, 2015 at 4:21 AM, wm4  wrote:
> On Thu,  8 Oct 2015 18:46:44 -0400
> Ganesh Ajjanagadde  wrote:
>
>> Note that for the current SDL source code, 0 is always returned.
>> Nevertheless, this makes the code more robust.
>>
>> Signed-off-by: Ganesh Ajjanagadde 
>> ---
>>  ffplay.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/ffplay.c b/ffplay.c
>> index 79f430d..c143e96 100644
>> --- a/ffplay.c
>> +++ b/ffplay.c
>> @@ -3814,7 +3814,8 @@ int main(int argc, char **argv)
>>  SDL_EventState(SDL_SYSWMEVENT, SDL_IGNORE);
>>  SDL_EventState(SDL_USEREVENT, SDL_IGNORE);
>>
>> -SDL_EnableKeyRepeat(SDL_DEFAULT_REPEAT_DELAY, 
>> SDL_DEFAULT_REPEAT_INTERVAL);
>> +if (SDL_EnableKeyRepeat(SDL_DEFAULT_REPEAT_DELAY, 
>> SDL_DEFAULT_REPEAT_INTERVAL) < 0)
>> +av_log(NULL, AV_LOG_ERROR, "SDL_EnableKeyRepeat(): %s\n", 
>> SDL_GetError());
>>
>>  if (av_lockmgr_register(lockmgr)) {
>>  av_log(NULL, AV_LOG_FATAL, "Could not initialize lock manager!\n");
>
> How does this make the code more "robust"?

If in future some limitations are placed on the repeat delay/repeat
interval apart from nonnegativity, or if someone accidentally changes
the repeat delay to e.g something dynamic and forgot to check that it
is > 0, etc we would get a log message from ffplay allowing quick
fixing of the issue.

I don't know if "robust" is the right word for it, maybe "more complete"?

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

Re: [FFmpeg-devel] [PATCH] ffplay: log possible error of SDL_EnableKeyRepeat

2015-10-09 Thread wm4 
On Thu,  8 Oct 2015 18:46:44 -0400
Ganesh Ajjanagadde  wrote:

> Note that for the current SDL source code, 0 is always returned.
> Nevertheless, this makes the code more robust.
> 
> Signed-off-by: Ganesh Ajjanagadde 
> ---
>  ffplay.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/ffplay.c b/ffplay.c
> index 79f430d..c143e96 100644
> --- a/ffplay.c
> +++ b/ffplay.c
> @@ -3814,7 +3814,8 @@ int main(int argc, char **argv)
>  SDL_EventState(SDL_SYSWMEVENT, SDL_IGNORE);
>  SDL_EventState(SDL_USEREVENT, SDL_IGNORE);
>  
> -SDL_EnableKeyRepeat(SDL_DEFAULT_REPEAT_DELAY, 
> SDL_DEFAULT_REPEAT_INTERVAL);
> +if (SDL_EnableKeyRepeat(SDL_DEFAULT_REPEAT_DELAY, 
> SDL_DEFAULT_REPEAT_INTERVAL) < 0)
> +av_log(NULL, AV_LOG_ERROR, "SDL_EnableKeyRepeat(): %s\n", 
> SDL_GetError());
>  
>  if (av_lockmgr_register(lockmgr)) {
>  av_log(NULL, AV_LOG_FATAL, "Could not initialize lock manager!\n");

How does this make the code more "robust"?
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH] ffplay: log possible error of SDL_EnableKeyRepeat

2015-10-08 Thread Ganesh Ajjanagadde 
Note that for the current SDL source code, 0 is always returned.
Nevertheless, this makes the code more robust.

Signed-off-by: Ganesh Ajjanagadde 
---
 ffplay.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/ffplay.c b/ffplay.c
index 79f430d..c143e96 100644
--- a/ffplay.c
+++ b/ffplay.c
@@ -3814,7 +3814,8 @@ int main(int argc, char **argv)
 SDL_EventState(SDL_SYSWMEVENT, SDL_IGNORE);
 SDL_EventState(SDL_USEREVENT, SDL_IGNORE);
 
-SDL_EnableKeyRepeat(SDL_DEFAULT_REPEAT_DELAY, SDL_DEFAULT_REPEAT_INTERVAL);
+if (SDL_EnableKeyRepeat(SDL_DEFAULT_REPEAT_DELAY, 
SDL_DEFAULT_REPEAT_INTERVAL) < 0)
+av_log(NULL, AV_LOG_ERROR, "SDL_EnableKeyRepeat(): %s\n", 
SDL_GetError());
 
 if (av_lockmgr_register(lockmgr)) {
 av_log(NULL, AV_LOG_FATAL, "Could not initialize lock manager!\n");
-- 
2.6.1

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