Le nonidi 9 germinal, an CCXXIII, Stephan Holljes a écrit :
> I hope this addresses the issues mentioned.
> I added a new label in case of failure in http_open() in favor of
> duplicated code (i.e. calling av_dict_free() multiple times). I copied
> the style from the other functions.
> 
> Signed-off-by: Stephan Holljes <klaxa1...@googlemail.com>

The patch looks good to me (acronym to learn: LGTM) as is or with the code
moved into a separate function point.

Except one more point that was not addressed earlier: if you want to add
comments to a Git patch email, you need to put them between the "---" and
the first "diff --git" lines. That is where the small stats "2 files
changed, 28 insertions(+), 1 deletion(-)" is already inserted, it is
considered as a simple comment too.

If you put them before the "---" line, it becomes part of the commit message
when the patch is applied from the mail. For the final version, maybe better
attach the patch or push it to a public clone.

At this point, I suspect you can consider this will be applied soon (better
give it maybe two days before asking for merging) and, if you want to
strengthen your application, make the patch even better, for example getting
it to work for input as well as output.

For that, I suspect it will be convenient for you to isolate the code in its
own function, that makes handling the variables and the failure code path
easier, so you probably better follow wm4's advice.

Regards,

-- 
  Nicolas George

Attachment: signature.asc
Description: Digital signature

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

Reply via email to