Hi Arun,
First of all - thank you for all the patches, they are getting better and
better
with every version.
Two things:
1) A minor one - I suggest that the choice of the default pixel format should
be
done in a different manner. I suggest using a V4L2_PIX_FMT_* in the
DEF_SRC_FMT_DEC
define and then using find_format in s5p_mfc_dec_init. Same goes for encoding.
2) A major one - the ops mechanism could be improved. I see that there are
many functions that only call an ops. Such as:
> int s5p_mfc_alloc_dec_temp_buffers(struct s5p_mfc_ctx *ctx)
> {
> return s5p_mfc_ops->s5p_mfc_alloc_dec_temp_buffers(ctx);
> }
I suggest dropping the s5p_mfc_ prefix in all the fields of s5p_mfc_hw_ops and
using a macro to call the ops. Like this:
+#define fimc_pipeline_call(f, op, p, args...) \
+ (!(f) ? -ENODEV : (((f)->pipeline_ops && (f)->pipeline_ops->op) ? \
+ (f)->pipeline_ops->op((p), ##args) :
-ENOIOCTLCMD))
(from commit by Sylwester Nawrocki http://goo.gl/Q657j)
In addition, your code does not check whether the ops pointer is null and I
think that it should be done.
Best wishes,
--
Kamil Debski
Linux Platform Group
Samsung Poland R&D Center
[snip the code]
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
