Re: [FFmpeg-devel] [PATCH V3] Add a filter implementing HDR image reconstruction from a single exposure using deep CNNs

2018-10-22 Thread Guo, Yejun


> -Original Message-
> From: Song, Ruiling
> Sent: Monday, October 22, 2018 10:02 PM
> To: FFmpeg development discussions and patches  de...@ffmpeg.org>; Guo, Yejun 
> Subject: RE: [FFmpeg-devel] [PATCH V3] Add a filter implementing HDR
> image reconstruction from a single exposure using deep CNNs
> 
> > Thanks for the link, however i'm still not sold on the term. You "generate"
> > hdr data, not "reconstruct": it's generated/estimated/made up data,
> > not data that is lost and needs to be reconstrcuted. I suggested "tonemap"
> > because you're mapping SDR tones (aka colors) to HDR ones, and that
> > seems the right term to use. If you really dislike it, at least
> > consider "HDR image generation from a single exposure using deep CNNs"
> > which would work
> I think "inverse/reverse tone mapping" looks a little better. I see many
> papers use this term when talking about sdr to hdr.
> 

I personally more like "HDR image generation from a single exposure using deep 
CNNs" to 
bind it loosely with the title of the paper.

> > much better.
> > --
> > Vittorio
> > ___
> > 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 V3] Add a filter implementing HDR image reconstruction from a single exposure using deep CNNs

2018-10-22 Thread Song, Ruiling
> Thanks for the link, however i'm still not sold on the term. You "generate"
> hdr data, not "reconstruct": it's generated/estimated/made up data, not
> data that is lost and needs to be reconstrcuted. I suggested "tonemap"
> because you're mapping SDR tones (aka colors) to HDR ones, and that seems
> the right term to use. If you really dislike it, at least consider "HDR
> image generation from a single exposure using deep CNNs" which would work
I think "inverse/reverse tone mapping" looks a little better. I see many papers 
use this term when talking about sdr to hdr.

> much better.
> --
> Vittorio
> ___
> 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 V3] Add a filter implementing HDR image reconstruction from a single exposure using deep CNNs

2018-10-21 Thread Vittorio Giovara
On Mon, Oct 22, 2018 at 4:19 AM Guo, Yejun  wrote:

> +.description   = NULL_IF_CONFIG_SMALL("HDR image reconstruction from
> a single exposure using deep CNNs."),
>
>
>
> > why "reconstruction"? there is nothing to construct back if the source
> wasn't hdr to begin with
>
> > "tonemap" is probably a better term here, in my opinion
>
> > same for previous uses
>
>
>
> there is more detail data generated with the dnn model, the model accepts
> sdr frame and generates hdr data.
>
> see more detail in paper @https://arxiv.org/pdf/1710.07480.pdf, and the
> description comes from the title of this paper.
>

Thanks for the link, however i'm still not sold on the term. You "generate"
hdr data, not "reconstruct": it's generated/estimated/made up data, not
data that is lost and needs to be reconstrcuted. I suggested "tonemap"
because you're mapping SDR tones (aka colors) to HDR ones, and that seems
the right term to use. If you really dislike it, at least consider "HDR
image generation from a single exposure using deep CNNs" which would work
much better.
-- 
Vittorio
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH V3] Add a filter implementing HDR image reconstruction from a single exposure using deep CNNs

2018-10-19 Thread Vittorio Giovara
On Fri, Oct 19, 2018 at 10:11 AM Guo, Yejun  wrote:

> see the algorithm's paper and code below.
>
> the filter's parameter looks like:
> sdr2hdr=model_filename=/path_to_tensorflow_graph.pb:out_fmt=gbrp10le
>

can you add some usage documentation to doc/filters.texi?

The input of the deep CNN model is RGB24 while the output is float
> for each color channel. This is the filter's default behavior to
> output format with gbrpf32le. And gbrp10le is also supported as the
> output, so we can see the rendering result in a player, as a reference.
>
> To generate the model file, we need modify the original script a little.
> - set name='y' for y_final within script at
> https://github.com/gabrieleilertsen/hdrcnn/blob/master/network.py
> - add the following code to the script at
> https://github.com/gabrieleilertsen/hdrcnn/blob/master/hdrcnn_predict.py
>
> graph = tf.graph_util.convert_variables_to_constants(sess, sess.graph_def,
> ["y"])
> tf.train.write_graph(graph, '.', 'graph.pb', as_text=False)
>
> The filter only works when tensorflow C api is supported in the system,
> native backend is not supported since there are some different types of
> layers in the deep CNN model, besides CONV and DEPTH_TO_SPACE.
>
> https://arxiv.org/pdf/1710.07480.pdf:
>   author   = "Eilertsen, Gabriel and Kronander, Joel, and Denes,
> Gyorgy and Mantiuk, RafaƂ and Unger, Jonas",
>   title= "HDR image reconstruction from a single exposure using
> deep CNNs",
>   journal  = "ACM Transactions on Graphics (TOG)",
>   number   = "6",
>   volume   = "36",
>   articleno= "178",
>   year = "2017"
>
> https://github.com/gabrieleilertsen/hdrcnn
>
> btw, as a whole solution, metadata should also be generated from
> the sdr video, so to be encoded as a HDR video. Not supported yet.
> This patch just focuses on this paper.
>

Is this something you are working on and will it be added later?


> v3: use int16_t instead of short
> v2: use AV_OPT_TYPE_PIXEL_FMT for filter option
> remove some unnecessary code
> Use in->linesize[0] and FFMAX/FFMIN
> remove flag AVFILTER_FLAG_SLICE_THREADS
> add av_log message when error
>

there is no need for this block to be left in the commit log


> Signed-off-by: Guo, Yejun 
> ---
>  libavfilter/Makefile |   1 +
>  libavfilter/allfilters.c |   1 +
>  libavfilter/vf_sdr2hdr.c | 266
> +++
>  3 files changed, 268 insertions(+)
>  create mode 100644 libavfilter/vf_sdr2hdr.c
>
> +static av_cold int init(AVFilterContext* context)
> +{
> +SDR2HDRContext* ctx = context->priv;
> +
> +if (ctx->out_fmt != AV_PIX_FMT_GBRPF32LE && ctx->out_fmt !=
> AV_PIX_FMT_GBRP10LE) {
> +av_log(context, AV_LOG_ERROR, "could not support the output
> format\n");
> +return AVERROR(ENOSYS);
> +}
> +
> +#if (CONFIG_LIBTENSORFLOW == 1)
> +ctx->dnn_module = ff_get_dnn_module(DNN_TF);
> +if (!ctx->dnn_module){
> +av_log(context, AV_LOG_ERROR, "could not create DNN module for
> tensorflow backend\n");
> +return AVERROR(ENOMEM);
> +}
> +if (!ctx->model_filename){
> +av_log(context, AV_LOG_ERROR, "model file for network was not
> specified\n");
> +return AVERROR(EIO);
> +}
> +if (!ctx->dnn_module->load_model) {
> +av_log(context, AV_LOG_ERROR, "load_model for network was not
> specified\n");
> +return AVERROR(EIO);
> +}
> +ctx->model = (ctx->dnn_module->load_model)(ctx->model_filename);
> +if (!ctx->model){
> +av_log(context, AV_LOG_ERROR, "could not load DNN model\n");
> +return AVERROR(EIO);
> +}
> +return 0;
> +#else
> +return AVERROR(EIO);
> +#endif
> +}
>

this is incorrect, what you should do is make libtensorflow a dependency of
this filter in the configure file and disable this filter when it is not
enabled


> +
> +static int query_formats(AVFilterContext* context)
> +{
> +const enum AVPixelFormat in_formats[] = {AV_PIX_FMT_RGB24,
> + AV_PIX_FMT_NONE};
> +enum AVPixelFormat out_formats[2];
> +SDR2HDRContext* ctx = context->priv;
> +AVFilterFormats* formats_list;
> +int ret = 0;
> +
> +formats_list = ff_make_format_list(in_formats);
> +if ((ret = ff_formats_ref(formats_list,
> >inputs[0]->out_formats)) < 0)
> +return ret;
> +
> +out_formats[0] = ctx->out_fmt;
> +out_formats[1] = AV_PIX_FMT_NONE;
> +formats_list = ff_make_format_list(out_formats);
> +if ((ret = ff_formats_ref(formats_list,
> >outputs[0]->in_formats)) < 0)
> +return ret;
> +
> +return 0;
> +}
> +
> +static int config_props(AVFilterLink* inlink)
> +{
> +AVFilterContext* context = inlink->dst;
> +SDR2HDRContext* ctx = context->priv;
> +AVFilterLink* outlink = context->outputs[0];
> +DNNReturnType result;
> +
> +// the dnn model is tied with resolution due to deconv layer of
> tensorflow
> +// now just support