Hi,
note: I'm using "step" throughout the patch because of the step
function and what the causal part most often looks like. I have no
idea for another less confusing wording.
2014-07-28 23:15 GMT+02:00 Michael Niedermayer <michae...@gmx.at>:
> maybe i misunderstand but the "progress[1] > y" check looks odd,
> shouldnt progress[0] be updated to a larger value even if
> progress[1] > y
> ?
Let's look at the following schema, that maybe should be added to the
documentation:
___ p[0]
p[1] ___|
p[2]
p(rogress)[0] and p[1] are the ordinates of the step. p[2] is the abscissa.
This is stranger than needed because I wanted progress[0] to keep its
meaning from previous API:
- you can somewhat easily revert back to this previous API way of working
- whether intentionally or erroneously, you can somewhat mix the 2 (eg
wait using previous API while using reporting using new one); this is
not a great benefit but I was wondering if in some cases you couldn't
avoid operating the 2 (for hevc, wpp/slice threading in a frame and
normal in another)
I think this makes the meaning of progress[] less obvious, but it had
some practicality at the time.
> from ff_thread_await_progress3()
> > + if (!progress || progress[0] >= y ||
> > + (progress[2] >= x && progress[1] >= y)) return;
>
>
> i would have expected report to do:
>
> if (progress[1] <= y + step && progress[2] <= x)
> update progress[1..2]
>
> but again, maybe iam missing something
I'm not sure how much the previous explanation changes your
understanding. I think you've been made fully aware by the comment in
the code, but that step parameter is only useful for the x/y progress,
and in a limited way. Sorry in advance if the following rationale is
more confusing than anything else.
The step parameter assumes square elements (blocks) are being decoded
and advance the progress.
_raster_end really does not care about the step: it just signals the
end of the raster line, thus the progress is just flat (no step).
The step parameter is only useful while in a step progress.
Now, the one concern with the step progress API is when concurrent
parts of a line are being decoded (think of a slice starting on this
line). This may break if slice threading is also activated. The
increment checking that uses the step tries to avoid this.
But in some cases (several wpp threads), the progress is more like
several staircases. Handling this would get out of hand and is less
practical, so we are only dealing with one at a time.
Nevertheless, when the raster line completes, the step progress on the
following line may be well underway because of this, and the step
increment check would force waiting for line completion, loosing any
benefit of the API. So the step parameter can be used to both:
- check the increment
- immediately update progress on next line to allow some progress in it.
> if progress[0] starts with 0 then this should be unneeded
It's not always 0 if not enough attention is being paid by the user.
Example with hevc: a block might have been completely
in-loop-filtered, while the right neighbour isn't. In that case, a
+/-4 pixels band around the top of the unfiltered neighbour can't
used. If we are on the start of the image, this makes it -4. We can
check this in the hevc code (clipping y position) but I thought it
would simpler to handle it inside of the API.
> also if the x and y step size is constant then one could use a
> single variable based progress with
> x_in_steps + y_in_steps * width_in_steps
Yes, Donald mentioned this and so people could probably rewrite codecs
to benefit from this finer granularity. My cursory look revealed
nothing regarding vp8/9 but I may be wrong.
However, I think it wouldn't allow the detail of the jumping position,
though. Regarding the constant position, most likely, yes.
A contrived counter-example are hevc post-filters that can be disabled
on a slice-basis. Currently that doesn't matter, because we only look
at the sps info, but it is possible to consider different steps
depending on the frame etc.
Overall, I don't think this new API is much future-proof, because the
threading options in ffhevc are limited. I tried to start looking at
vp9 but I realistically don't have that much time and will to come
with a working version in that case.
--
Christophe
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
