Ambiguity in checking the resolution capability of an OMX decoder

RESOLVED WONTFIX

Status

RESOLVED WONTFIX
5 years ago
8 months ago

People

(Reporter: brsun, Unassigned)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Reporter)

Description

5 years ago
There are two blocks of codes to check OMX decoder capability with the required frame size:
 - During the creation of an OMX decoder component, we need to configure input port and output port of it. If IOMX::setParameter() fails to configure resolution of input/output port, the created OMXCodec object will be destroyed.
 - After the created OMXCodec object can be configured successfully, we will use values of "ro.moz.omx.hw.max_width" and "ro.moz.omx.hw.max_height" to check with the required frame width/height. If the required frame width*height exceeds predefined max_width*max_height, we will not use this OMXCodec object for decoding.

The logic of the second block of codes seems like some porting/customization of underlying HW codec. Is it good to move this logic into underlying OMX component so that we can just use the first block of codes to check the capability of the decoder? Or the second logic is required to avoid some error conditions which cannot be checked through IOMX::setPrameter() API?

Kindly share your information on this.

// First block of codes:
status_t OMXCodec::setVideoOutputFormat(const char *mime, OMX_U32 width, OMX_U32 height) {
  .....
  OMX_PARAM_PORTDEFINITIONTYPE def;
  InitOMXParams(&def);
  def.nPortIndex = kPortIndexInput;
  .....
  video_def->nFrameWidth = width;
  video_def->nFrameHeight = height;
  .....
  err = mOMX->setParameter(mNode, OMX_IndexParamPortDefinition, &def, sizeof(def));
  if (err != OK) {
    return err;
  }
  .....
  InitOMXParams(&def);
  def.nPortIndex = kPortIndexOutput;
  .....
  video_def->nFrameWidth = width;
  video_def->nFrameHeight = height;
  err = mOMX->setParameter(mNode, OMX_IndexParamPortDefinition, &def, sizeof(def));
  return err;
}

// Second block of codes:
void OMXCodecProxy::statusChanged(int event)
{
  .....
  if (!strncasecmp(mime, "video/", 6)) {
    .....
    // Check if this video is sized such that we're comfortable
    // possibly using an OMX decoder.
    int32_t maxWidth, maxHeight;
    char propValue[PROPERTY_VALUE_MAX];
    property_get("ro.moz.omx.hw.max_width", propValue, "-1");
    maxWidth = atoi(propValue);
    property_get("ro.moz.omx.hw.max_height", propValue, "-1");
    maxHeight = atoi(propValue);

    int32_t width = -1, height = -1;
    if (maxWidth > 0 && maxHeight > 0 &&
        !(mOMXCodec->getFormat()->findInt32(kKeyWidth, &width) &&
          mOMXCodec->getFormat()->findInt32(kKeyHeight, &height) &&
          width * height <= maxWidth * maxHeight)) {
      printf_stderr("Failed to get video size, or it was too large for HW decoder (<w=%d, h=%d> but <maxW=%d, maxH=%d>)",
                    width, height, maxWidth, maxHeight);
      mOMXCodec.clear();
      mState = MediaResourceManagerClient::CLIENT_STATE_SHUTDOWN;
      notifyStatusChangedLocked();
      return;
    }
    .....
  }
 .....
}
Bug 829361 comment #3 explains about it. Even when Hw codec can support the some large size video, there is a cases that needs to limit the size of decoding like resource limitation. OMXCodec is outside of gecko, keep "ro.moz.omx.hw.max_width" in gecko is convenient. It might be better to add a comment here.

Comment 2

5 years ago
Is there any place to note this kind of thing? It needs every partner to put correct value on their different devices equipped different resources.

Basically I saw it it VGA on Leo and WVGA on Helix. Does the limitation is always device's screen size?

Thanks.
(In reply to Marco Chen [:mchen] from comment #2)
> Basically I saw it it VGA on Leo and WVGA on Helix. Does the limitation is
> always device's screen size?

It should be decided by partners. Actually the product need to support at least device's screen size, otherwise it could play only blurry videos.
(In reply to Sotaro Ikeda [:sotaro] from comment #3)
> 
> It should be decided by partners. Actually the product need to support at
> least device's screen size, otherwise it could play only blurry videos.

Correction;
Video becomes always blurry when play backed in fullscreen, if the product support smaller video side than device's screen size.
(In reply to Marco Chen [:mchen] from comment #2)
> Is there any place to note this kind of thing? It needs every partner to put
> correct value on their different devices equipped different resources.

:mwu, is there a place in b2g source for gathering and explaining the b2g specific property like "ro.moz.omx.hw.max_width".
Flags: needinfo?(mwu)

Comment 6

5 years ago
(In reply to Sotaro Ikeda [:sotaro] from comment #5)
> :mwu, is there a place in b2g source for gathering and explaining the b2g
> specific property like "ro.moz.omx.hw.max_width".

Not that I know of, but we should create one. I would create a wiki.mozilla page for it - not sure if there's a good place in the source for it since any piece of code can query those properties.
Flags: needinfo?(mwu)
This bug is hit a couple of times when porting to some new devices and spend some R&D resource to debug it. It would be better to remove it. AFAIK, Android doesn't have this kind of check. 
If this check is really required, partners can add it to their OMX HAL to return an error when setting parameter as first block codes mentioned comment 0.

Comment 8

8 months ago
Firefox OS is not being worked on
Status: NEW → RESOLVED
Last Resolved: 8 months ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.