| Message ID | 20210303222722.14368-1-cus@passwd.hu |
|---|---|
| State | Accepted |
| Commit | 573f05a7533cd9aed3ed895b4fa4ad8fcba4e56a |
| Headers | show |
| Series | [FFmpeg-devel] fftools/ffplay: do not write out of rdft visualization texture | expand |
| Context | Check | Description |
|---|---|---|
| andriy/x86_make | success | Make finished |
| andriy/x86_make_fate | success | Make fate finished |
| andriy/PPC64_make | success | Make finished |
| andriy/PPC64_make_fate | success | Make fate finished |
On Wed, Mar 03, 2021 at 11:27:22PM +0100, Marton Balint wrote: > If the window is resized it was possible that xpos pointed outside the > visualization texture. By rearranging the overflow check we make sure this (and > a crash) does not happen. > > We also don't have to use xleft for start position, as that is 0 anyways, and > if we ever want to take into account xleft then the texture should be > positioned accordingly when rendering. reading this, i wonder if a assertion with xleft == 0 would make sense thx [...]
On Sun, 7 Mar 2021, Michael Niedermayer wrote: > On Wed, Mar 03, 2021 at 11:27:22PM +0100, Marton Balint wrote: >> If the window is resized it was possible that xpos pointed outside the >> visualization texture. By rearranging the overflow check we make sure this (and >> a crash) does not happen. >> > >> We also don't have to use xleft for start position, as that is 0 anyways, and >> if we ever want to take into account xleft then the texture should be >> positioned accordingly when rendering. > > reading this, i wonder if a assertion with xleft == 0 would make sense I don't really see the point. I'd rather add the xleft/ytop to the render if you prefer, but overall I don't think it matters. --- a/fftools/ffplay.c +++ b/fftools/ffplay.c @@ -1193,7 +1193,8 @@ static void video_audio_display(VideoState *s) } SDL_UnlockTexture(s->vis_texture); } - SDL_RenderCopy(renderer, s->vis_texture, NULL, NULL); + rect = (SDL_Rect){s->xleft, s->ytop, s->width, s->height}; + SDL_RenderCopy(renderer, s->vis_texture, NULL, &rect); } if (!s->paused) s->xpos++; Regards, Marton
On Mon, Mar 08, 2021 at 09:56:38PM +0100, Marton Balint wrote: > > > On Sun, 7 Mar 2021, Michael Niedermayer wrote: > > > On Wed, Mar 03, 2021 at 11:27:22PM +0100, Marton Balint wrote: > > > If the window is resized it was possible that xpos pointed outside the > > > visualization texture. By rearranging the overflow check we make sure this (and > > > a crash) does not happen. > > > > > > > > We also don't have to use xleft for start position, as that is 0 anyways, and > > > if we ever want to take into account xleft then the texture should be > > > positioned accordingly when rendering. > > > > reading this, i wonder if a assertion with xleft == 0 would make sense > > I don't really see the point. It was just an idea that came to my mind without any deep thoughts > I'd rather add the xleft/ytop to the render if > you prefer, but overall I don't think it matters. please do what you prefer! thx [...]
On Mon, 8 Mar 2021, Michael Niedermayer wrote: > On Mon, Mar 08, 2021 at 09:56:38PM +0100, Marton Balint wrote: >> >> >> On Sun, 7 Mar 2021, Michael Niedermayer wrote: >> >>> On Wed, Mar 03, 2021 at 11:27:22PM +0100, Marton Balint wrote: >>>> If the window is resized it was possible that xpos pointed outside the >>>> visualization texture. By rearranging the overflow check we make sure this (and >>>> a crash) does not happen. >>>> >>> >>>> We also don't have to use xleft for start position, as that is 0 anyways, and >>>> if we ever want to take into account xleft then the texture should be >>>> positioned accordingly when rendering. >>> >>> reading this, i wonder if a assertion with xleft == 0 would make sense >> >> I don't really see the point. > > It was just an idea that came to my mind without any deep thoughts > > >> I'd rather add the xleft/ytop to the render if >> you prefer, but overall I don't think it matters. > > please do what you prefer! Ok, thanks, pushed as is then. Regards, Marton
diff --git a/fftools/ffplay.c b/fftools/ffplay.c index b9a30cdb11..0cdadab293 100644 --- a/fftools/ffplay.c +++ b/fftools/ffplay.c @@ -1148,6 +1148,8 @@ static void video_audio_display(VideoState *s) if (realloc_texture(&s->vis_texture, SDL_PIXELFORMAT_ARGB8888, s->width, s->height, SDL_BLENDMODE_NONE, 1) < 0) return; + if (s->xpos >= s->width) + s->xpos = 0; nb_display_channels= FFMIN(nb_display_channels, 2); if (rdft_bits != s->rdft_bits) { av_rdft_end(s->rdft); @@ -1197,8 +1199,6 @@ static void video_audio_display(VideoState *s) } if (!s->paused) s->xpos++; - if (s->xpos >= s->width) - s->xpos= s->xleft; } }
If the window is resized it was possible that xpos pointed outside the visualization texture. By rearranging the overflow check we make sure this (and a crash) does not happen. We also don't have to use xleft for start position, as that is 0 anyways, and if we ever want to take into account xleft then the texture should be positioned accordingly when rendering. Signed-off-by: Marton Balint <cus@passwd.hu> --- fftools/ffplay.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)