Re: [FFmpeg-devel] [PATCH] ffplay: more robust mutex, condition variable handling

[Ganesh Ajjanagadde]

On Thu, Oct 1, 2015 at 7:16 AM, Ganesh Ajjanagadde
 wrote:
> On Sep 30, 2015 7:50 PM, "Marton Balint"  wrote:
>>
>>
>> On Tue, 29 Sep 2015, Ganesh Ajjanagadde wrote:
>>
>>> SDL_CreateMutex and SDL_CreateCond can fail:
>>> https://wiki.libsdl.org/SDL_CreateMutex.
>>> This patch makes handling more robust in one instance.
>>>
>>> Signed-off-by: Ganesh Ajjanagadde 
>>> ---
>>> ffplay.c | 17 +
>>> 1 file changed, 13 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/ffplay.c b/ffplay.c
>>> index 3c2407f..9466996 100644
>>> --- a/ffplay.c
>>> +++ b/ffplay.c
>>> @@ -451,12 +451,21 @@ static int packet_queue_put_nullpacket(PacketQueue
>>> *q, int stream_index)
>>> }
>>>
>>> /* packet queue handling */
>>> -static void packet_queue_init(PacketQueue *q)
>>> +static int packet_queue_init(PacketQueue *q)
>>> {
>>> memset(q, 0, sizeof(PacketQueue));
>>> q->mutex = SDL_CreateMutex();
>>> +if (!q->mutex) {
>>> +av_log(q, AV_LOG_FATAL, "SDL_CreateMutex(): %s\n",
>>> SDL_GetError());
>>> +return AVERROR(ENOMEM);
>>> +}
>>> q->cond = SDL_CreateCond();
>>> +if (!q->cond) {
>>> +av_log(q, AV_LOG_FATAL, "SDL_CreateCond(): %s\n",
>>> SDL_GetError());
>>> +return AVERROR(ENOMEM);
>>> +}
>>> q->abort_request = 1;
>>> +return 0;
>>> }
>>>
>>> static void packet_queue_flush(PacketQueue *q)
>>> @@ -3136,9 +3145,9 @@ static VideoState *stream_open(const char
>>> *filename, AVInputFormat *iformat)
>>> if (frame_queue_init(>sampq, >audioq, SAMPLE_QUEUE_SIZE, 1) <
>>> 0)
>>> goto fail;
>>>
>>> -packet_queue_init(>videoq);
>>> -packet_queue_init(>audioq);
>>> -packet_queue_init(>subtitleq);
>>> +if (packet_queue_init(>videoq) || packet_queue_init(>audioq)
>>> +|| packet_queue_init(>subtitleq))
>>> +goto fail;
>>
>>
>> Only cosmetics, but maybe you could use ffmpeg-api-like less-than-zero
>> error checking with common indentation:
>>
>> if (packet_queue_init() < 0 ||
>> packet_queue_init() < 0 ||
>> packet_queue_init() < 0)
>
> Thanks for the style tip. I am currently away from a machine, will get to it
> in a few days.

Changed style and updated patch.

>
>>
>> Regards,
>> Marton
>> ___
>> 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: more robust mutex, condition variable handling

[Ganesh Ajjanagadde]

On Sep 30, 2015 7:50 PM, "Marton Balint"  wrote:
>
>
> On Tue, 29 Sep 2015, Ganesh Ajjanagadde wrote:
>
>> SDL_CreateMutex and SDL_CreateCond can fail:
>> https://wiki.libsdl.org/SDL_CreateMutex.
>> This patch makes handling more robust in one instance.
>>
>> Signed-off-by: Ganesh Ajjanagadde 
>> ---
>> ffplay.c | 17 +
>> 1 file changed, 13 insertions(+), 4 deletions(-)
>>
>> diff --git a/ffplay.c b/ffplay.c
>> index 3c2407f..9466996 100644
>> --- a/ffplay.c
>> +++ b/ffplay.c
>> @@ -451,12 +451,21 @@ static int packet_queue_put_nullpacket(PacketQueue
*q, int stream_index)
>> }
>>
>> /* packet queue handling */
>> -static void packet_queue_init(PacketQueue *q)
>> +static int packet_queue_init(PacketQueue *q)
>> {
>> memset(q, 0, sizeof(PacketQueue));
>> q->mutex = SDL_CreateMutex();
>> +if (!q->mutex) {
>> +av_log(q, AV_LOG_FATAL, "SDL_CreateMutex(): %s\n",
SDL_GetError());
>> +return AVERROR(ENOMEM);
>> +}
>> q->cond = SDL_CreateCond();
>> +if (!q->cond) {
>> +av_log(q, AV_LOG_FATAL, "SDL_CreateCond(): %s\n",
SDL_GetError());
>> +return AVERROR(ENOMEM);
>> +}
>> q->abort_request = 1;
>> +return 0;
>> }
>>
>> static void packet_queue_flush(PacketQueue *q)
>> @@ -3136,9 +3145,9 @@ static VideoState *stream_open(const char
*filename, AVInputFormat *iformat)
>> if (frame_queue_init(>sampq, >audioq, SAMPLE_QUEUE_SIZE, 1)
< 0)
>> goto fail;
>>
>> -packet_queue_init(>videoq);
>> -packet_queue_init(>audioq);
>> -packet_queue_init(>subtitleq);
>> +if (packet_queue_init(>videoq) || packet_queue_init(>audioq)
>> +|| packet_queue_init(>subtitleq))
>> +goto fail;
>
>
> Only cosmetics, but maybe you could use ffmpeg-api-like less-than-zero
error checking with common indentation:
>
> if (packet_queue_init() < 0 ||
> packet_queue_init() < 0 ||
> packet_queue_init() < 0)

Thanks for the style tip. I am currently away from a machine, will get to
it in a few days.

>
> Regards,
> Marton
> ___
> 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: more robust mutex, condition variable handling

[Marton Balint]

On Tue, 29 Sep 2015, Ganesh Ajjanagadde wrote:


SDL_CreateMutex and SDL_CreateCond can fail:
https://wiki.libsdl.org/SDL_CreateMutex.
This patch makes handling more robust in one instance.

Signed-off-by: Ganesh Ajjanagadde 
---
ffplay.c | 17 +
1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/ffplay.c b/ffplay.c
index 3c2407f..9466996 100644
--- a/ffplay.c
+++ b/ffplay.c
@@ -451,12 +451,21 @@ static int packet_queue_put_nullpacket(PacketQueue *q, 
int stream_index)
}

/* packet queue handling */
-static void packet_queue_init(PacketQueue *q)
+static int packet_queue_init(PacketQueue *q)
{
memset(q, 0, sizeof(PacketQueue));
q->mutex = SDL_CreateMutex();
+if (!q->mutex) {
+av_log(q, AV_LOG_FATAL, "SDL_CreateMutex(): %s\n", SDL_GetError());
+return AVERROR(ENOMEM);
+}
q->cond = SDL_CreateCond();
+if (!q->cond) {
+av_log(q, AV_LOG_FATAL, "SDL_CreateCond(): %s\n", SDL_GetError());
+return AVERROR(ENOMEM);
+}
q->abort_request = 1;
+return 0;
}

static void packet_queue_flush(PacketQueue *q)
@@ -3136,9 +3145,9 @@ static VideoState *stream_open(const char *filename, 
AVInputFormat *iformat)
if (frame_queue_init(>sampq, >audioq, SAMPLE_QUEUE_SIZE, 1) < 0)
goto fail;

-packet_queue_init(>videoq);
-packet_queue_init(>audioq);
-packet_queue_init(>subtitleq);
+if (packet_queue_init(>videoq) || packet_queue_init(>audioq)
+|| packet_queue_init(>subtitleq))
+goto fail;


Only cosmetics, but maybe you could use ffmpeg-api-like less-than-zero 
error checking with common indentation:


if (packet_queue_init() < 0 ||
packet_queue_init() < 0 ||
packet_queue_init() < 0)

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

Re: [FFmpeg-devel] [PATCH] ffplay: more robust mutex, condition variable handling

[Ganesh Ajjanagadde]

On Sun, Sep 27, 2015 at 9:35 PM, Ganesh Ajjanagadde
 wrote:
> SDL_CreateMutex and SDL_CreateCond can fail:
> https://wiki.libsdl.org/SDL_CreateMutex.
> This patch makes handling more robust in one instance.
>
> Signed-off-by: Ganesh Ajjanagadde 
> ---
>  ffplay.c | 23 +--
>  1 file changed, 17 insertions(+), 6 deletions(-)
>
> diff --git a/ffplay.c b/ffplay.c
> index 10887e1..27af336 100644
> --- a/ffplay.c
> +++ b/ffplay.c
> @@ -451,12 +451,21 @@ static int packet_queue_put_nullpacket(PacketQueue *q, 
> int stream_index)
>  }
>
>  /* packet queue handling */
> -static void packet_queue_init(PacketQueue *q)
> +static int packet_queue_init(PacketQueue *q)
>  {
>  memset(q, 0, sizeof(PacketQueue));
>  q->mutex = SDL_CreateMutex();
> +if (!q->mutex) {
> +av_log(q, AV_LOG_FATAL, "SDL_CreateMutex(): %s\n", SDL_GetError());
> +return AVERROR(EINVAL);
> +}
>  q->cond = SDL_CreateCond();
> +if (!q->cond) {
> +av_log(q, AV_LOG_FATAL, "SDL_CreateCond(): %s\n", SDL_GetError());
> +return AVERROR(EINVAL);
> +}
>  q->abort_request = 1;
> +return 0;
>  }
>
>  static void packet_queue_flush(PacketQueue *q)
> @@ -479,8 +488,10 @@ static void packet_queue_flush(PacketQueue *q)
>  static void packet_queue_destroy(PacketQueue *q)
>  {
>  packet_queue_flush(q);
> -SDL_DestroyMutex(q->mutex);
> -SDL_DestroyCond(q->cond);
> +if (q->mutex)
> +SDL_DestroyMutex(q->mutex);
> +if (q->cond)
> +SDL_DestroyCond(q->cond);
>  }
>
>  static void packet_queue_abort(PacketQueue *q)
> @@ -3136,9 +3147,9 @@ static VideoState *stream_open(const char *filename, 
> AVInputFormat *iformat)
>  if (frame_queue_init(>sampq, >audioq, SAMPLE_QUEUE_SIZE, 1) < 0)
>  goto fail;
>
> -packet_queue_init(>videoq);
> -packet_queue_init(>audioq);
> -packet_queue_init(>subtitleq);
> +if (!packet_queue_init(>videoq) || !packet_queue_init(>audioq)
> +|| packet_queue_init(>subtitleq))
> +goto fail;
>
>  is->continue_read_thread = SDL_CreateCond();
>
> --
> 2.5.3
>

please ignore this - wrong patch. Apologies.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel