Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

lib/metrics: support mul-res ssim/md5 calculation #24

Closed
wants to merge 2 commits into from

Conversation

wangzj0601
Copy link
Contributor

@wangzj0601 wangzj0601 commented Feb 27, 2019

support multi-resolution ssim/md5 calculation

user should define the case with param "multi_res" as below:
multi_res = [(width1, height1, frames1), (width2, height2, frames2), ...]

Signed-off-by: Wang Zhanjun zhanjunx.wang@intel.com

  support multi-resolution ssim calculation

  user should define the case with param "multi_res" as below:
  multi_res = [(width1, height1, frames1), (width2, height2, frames2), ...]

Signed-off-by: Wang Zhanjun <zhanjunx.wang@intel.com>
  support multi-resolution md5 calculation

  user should define the case with param "multi_res" as below:
  multi_res = [(width1, height1, frames1), (width2, height2, frames2), ...]

Signed-off-by: Wang Zhanjun <zhanjunx.wang@intel.com>
@wangzj0601 wangzj0601 changed the title lib/metrics: support mul-res ssim calculation lib/metrics: support mul-res ssim/md5 calculation Feb 28, 2019
@wangzj0601
Copy link
Contributor Author

can you have a reviw @uartie

Copy link
Contributor

@uartie uartie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if we wrote a solution that works with something like this in user config:

  "my_test_case_name" : dict(
    source = os.path.join(assets, "my_test_case_source.h264"),
    reference = os.path.join(assets, "yuv", "my_test_case_ref.yuv"),
    format = "I420", frames = 250,
    resolution = [
      dict(width = 1920, height = 1080, frames = 150),
      dict(width = 720, height = 480, frames = 100),
    ]
  ),

That is, in user config, we should replace width and height keys with a resolution key that is a list of dict values that can specify one or more width/height for the streams. The sum of frames in the resolution spec should equal the value of frames in outer spec.

@uartie
Copy link
Contributor

uartie commented Mar 5, 2019

Unfortunately, I think ffmpeg always scales the output to a single resolution (and it can't be disabled, afaict). Thus, this patch would break metrics on ffmpeg tests. Maybe it's better to insert the vaapi scaler into the gstreamer decode test pipelines and change ffmpeg decode tests to use the vaapi scaler (instead of sw scaler) so we get the same behavior and a single output resolution across all middleware decode tests.

@wangzj0601
Copy link
Contributor Author

wangzj0601 commented Mar 6, 2019

What if we wrote a solution that works with something like this in user config:

  "my_test_case_name" : dict(
    source = os.path.join(assets, "my_test_case_source.h264"),
    reference = os.path.join(assets, "yuv", "my_test_case_ref.yuv"),
    format = "I420", frames = 250,
    resolution = [
      dict(width = 1920, height = 1080, frames = 150),
      dict(width = 720, height = 480, frames = 100),
    ]
  ),

That is, in user config, we should replace width and height keys with a resolution key that is a list of dict values that can specify one or more width/height for the streams. The sum of frames in the resolution spec should equal the value of frames in outer spec.

I keep thewidth and height keys in config because the output name will use these params, if modify it as you said, we will change our test scripts.

@wangzj0601
Copy link
Contributor Author

Unfortunately, I think ffmpeg always scales the output to a single resolution (and it can't be disabled, afaict). Thus, this patch would break metrics on ffmpeg tests. Maybe it's better to insert the vaapi scaler into the gstreamer decode test pipelines and change ffmpeg decode tests to use the vaapi scaler (instead of sw scaler) so we get the same behavior and a single output resolution across all middleware decode tests.

I think that this is a ffmpeg enhancement issue, we should create an issue against ffmpeg. Of cause, we also can add "videoscale ! video/x-raw,width={width},height={height}" to the gstreamer at now.

@uartie
Copy link
Contributor

uartie commented Mar 6, 2019

What if we wrote a solution that works with something like this in user config:

  "my_test_case_name" : dict(
    source = os.path.join(assets, "my_test_case_source.h264"),
    reference = os.path.join(assets, "yuv", "my_test_case_ref.yuv"),
    format = "I420", frames = 250,
    resolution = [
      dict(width = 1920, height = 1080, frames = 150),
      dict(width = 720, height = 480, frames = 100),
    ]
  ),

That is, in user config, we should replace width and height keys with a resolution key that is a list of dict values that can specify one or more width/height for the streams. The sum of frames in the resolution spec should equal the value of frames in outer spec.

I keep thewidth and height keys in config because the output name will use these params, if modify it as you said, we will change our test scripts.

Well, yes, of course, those would have to be updated with such a change. What's wrong with that?

@uartie
Copy link
Contributor

uartie commented Mar 6, 2019

Unfortunately, I think ffmpeg always scales the output to a single resolution (and it can't be disabled, afaict). Thus, this patch would break metrics on ffmpeg tests. Maybe it's better to insert the vaapi scaler into the gstreamer decode test pipelines and change ffmpeg decode tests to use the vaapi scaler (instead of sw scaler) so we get the same behavior and a single output resolution across all middleware decode tests.

I think that this is a ffmpeg enhancement issue, we should create an issue against ffmpeg. Of cause, we also can add "videoscale ! video/x-raw,width={width},height={height}" to the gstreamer at now.

Yes, but we don't break existing tests intentionally either.

Let's settle on adding vaapi output scaling (not sw scaling) to the ffmpeg and gstreamer decode test pipelines for now. Pure multi-res support has a long way to go before I would be willing to accept such a change here.

@uartie uartie closed this Mar 6, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants