[Jellyfin] Render ahead (oneshot render)

Original author: @JustAMan
This PR adds a Render ahead mode that improves the accuracy of event timing by rendering a set of images in advance so that the frame is ready in time.
Jellyfin uses this mode.
I cherry-picked related commits, skipping merge-commits.

List of commits
848a686d8ca80dcc753ffcc8f07ebdb5591e105b
65fbfecfce503b001f55e0601c00ffa0b1f14c12
5430cc9823ca45bef41ef370eb0afef9cd078f53
4e4250204dd9b95bdc1bb3ecb67f27f587b8767e
90910bdef6e4dad06e34cfa417796583bc35cf46
21c84cc8038ea4da41652bbf22a47f18a6ead9a5
6980debc4712239f2a3c481a3a6756eca2642aca
6d3a5c731f52ea84282e40bed3014ef95220146a
7fc34cf58737992f92eb55df4d0d51b2740198e3
cea2dc9dee3d6c7607c7ec0dd48855ab59880b5a
88d2df425538467eba8d18ffc4cf1b9b3d3407f6
bd8bcab764177f5757007e606547df7397648aaa
ecd47dd25fdc7286d0bb554e99fd85c29dbc35dd
67947f395d9c6029f201e0dcc262d5b150ef1622
9dd6eb2411a18f81ad7c366224b449e6d34fb2e4
e1604cb133ef998e61e85332513301e1de008d10
Merge pull request #6 from JustAMan/improve-timing-accuracy
a8d670eb0955c81c6e07c99e28adac5e26c9f216
d3bc4724a0ce07f7cd02194834222999d7160a18
90d48486f1f0af0db4f1e0b17653b101781636ac
a66e79768b00b50d5a7e9651d2410677d346d688
971a9979517738ec1847350cb057d61c8840d605
a1666065e4980a7111a11817308941c4d4019560
Merge pull request #8 from JustAMan/lite-mode
825c9991598b33bf7f92df5c581d39d427cb6cba
345d7010b1d31f933f1b2cc3a17d83a09b5e5c2e
4c5f018a07d5c51b089ef21ccf1f81a0355e6c8c
bcf4b5fc62140a3ab37ce5e3f6c329c3094cb7d8
5256b6364941f7f77b547493141d59c9efc89d0d
Merge pull request #11 from JustAMan/limit-canvas-size
cc9575cd5137c97c347940bc7f9d54822358c133
93f4d0f4a77f60133a6db4b60dc7f135b7218daf
0722559b28ea3e9e1eeedc28b7f751439f345563
Merge pull request #12 from JustAMan/fix-resize
7eefe74ca28fd4fd46e1d9db475f2dc7f110aa42
595d0e20e58634130c237a2853be70fa193c530d
Merge pull request #15 from JustAMan/update-build
c66e7b1841118f957301e5b950d31c5e9aa32b6e
Merge pull request #19 from Teranode/master
0e9e7929dddbbf6168f43032ff90f6b86fd5c318
9eac714a26d6d57b83ed1274d7a45da160786710

Changes
Add Render ahead mode.

@dmitrylyzo could you resolve conflicts so we can get this moving?

renderAhead in README claims to be directly related to frame count; but the commment in subtitles-octopus.js says it relates to how many MiB's of data get (pre-rendered and) chached for later use. Which one is correct?

From the option list in README:

renderAhead: How many MiB (approximate) of subtitles to render ahead and store.

The added description of renderAhead reads to me like it is supposed to just prerender all timespans which ahve any event; but the animation bugs you uncovered earlier — which iiuc appeared when using the renderAhead mode — and a glance at libassjs_find_event_stop_times suggest it also tries to be smart regarding animated events. Can you clarify what it actually does (and amend the documentioan commit accordingly)?

Hmm, timespan is a more correct term.
It builds a set of timespans where nothing changes. As I understand.
Dialogue: 0,0:00:00.00,0:00:05.00,Default,,0000,0000,0000,,1
Dialogue: 0,0:00:03.00,0:00:08.00,Default,,0000,0000,0000,,2

There will be 3 timespans (by design): 1, 1+2, 2.
In fact, there are more:
0: 0..3                       1
1: 2.999..3.0323333333333333
2: 3.0323333333333333..5      1+2
3: 4.999..5.032333333333333
4: 5.032333333333333..8       2

The timespans in between are because of this time shifting (not sure of its purpose): https://github.com/libass/JavascriptSubtitlesOctopus/blob/58306888a286d8e9680e6290f6fc99d5cb7ceb56/src/subtitles-octopus.js#L330
Next, it needs multiple timespans for animation. Because of that bug, the oneshot renderer doesn't know it should make them, and only does the first one.

and in fact I don't know why ; are treated special at all, afaik they only have a special meaning inside the Effect field not in override-tags

I just left it as it was used. But I didn't see it in the documentation.

I suggest to only check for the backslash followed by the name of an override tag and not any of the subsequent cahracters.

Well, if only to expect correctly written tag names. Or maybe I can find the end of the word :thinking:
{\t (\frz360} - doesn't the missed ) bracket make sense?

It builds a set of timespans where nothing changes. As I understand.
[…]
Next, it needs multiple timespans for animation. Because of that bug, the oneshot renderer doesn't know it should make them, and only does the first one.

So yes, it does try to be clever (and partially fails). Putting aside if that even makes sense to begin with (libass also caches parts of events, more for non-animated then for animated), how about replacing the second-last sentence in #### Render Ahead with:
This mode will analyse the events to avoid rendering empty sections or rerendering non-animated events.

Well, if only to expect correctly written tag names. Or maybe I can find the end of the word :thinking:

The examples I gave are correct ASS. I think just not checking following characters is the safest, if you really want to I think checking for either ( or   to follow maybe works for complex tags (which require multiple arguments), but I'm not entirely sure about that and it definitely does not work for non-complex tags. (So again, not checking that is safest and false-positives should be sufficiently rare.)

{\t (\frz360} - doesn't the missed ) bracket make sense?

The example is correct, if it extends to the end of the override block it doesn't need a closing parenthesis ).

Looking at the revised “Fix event tag parsing” I can again see more parsing mistakes in the newly touched code (eg using locale-dependent functions, assuming parentheses can be nested, and more). Rather than analysing and fixing all of this I rewrote the tag parser, please replace the “Fix event tag parsing” commit with this one: fix-animation-detection.patch (apply on top of 8159a6b with git am <patch-file>).
I tested it on this file: animated.ass and for reference here's the expected and produced output with 0 for unanimated, 1 for animatedandclear` shows what dropping the animations does:
1:     Move (with tabulator) {\move (50,50,1230,670)}
clear: Move (with tabulator) {                      }
0:     {\c1&H000000&\alpha&HAA}Move (with unclosed block) {\move(50,50,1230,670)
1:     {\notatag \fs50 \t     (\frz360}Transform (with space)
clear: {\notatag \fs50         \frz360}Transform (with space)
1:     {\frz30 aa \fad(100,500) \fs120}Fad simple
clear: {\frz30 aa               \fs120}Fad simple
1:     {\frz-30 b \fade(50,255,0,10,300,700,900) \fs121}Fade complex
clear: {\frz-30 b                                \fs121}Fade complex
1:     いろは{\kf128}笑{\kf47}え{\kf24}ば {\kf91}空{\kf58}ま{\kf27}で
clear: いろは{      }笑{     }え{     }ば {     }空{     }ま{     }で
1:     にほえと{\ko128}笑{\ko47}え{\ko24}ば {\ko91}空{\ko58}ま{\ko27}で
clear: にほえと{      }笑{     }え{     }ば {     }空{     }ま{     }で
1:     ちりぬるを{\K128}笑{\k47}え{\K24}ば {\k91}空{\K58}ま{\k27}で
clear: ちりぬるを{     }笑{    }え{    }ば {    }空{    }ま{    }で
0:     {\shad5\bord5\fnIPAMincho}週間
0:     {\an2}Not animated due to libass ext \{\move(1,1,500,500)}
1:     {\an8}{\bord3}Animated {\move(1,1,500,500)} with unusual{\1c&H00FF00&}ly placed {\r}tags
clear: {\an8}{\bord3}Animated {                  } with unusual{\1c&H00FF00&}ly placed {\r}tags

(As a side note, assuming emscripten could utilise SIMD in its library function it would probably be more performant to adjust _is_event_aniamted and _is_block_animated to use strchr rather than a for-loop over all chars. But last I heard, WASM-SIMD is still not part of the standard but a browser-specific extension, so this probably doesn't matter — and I'm not sure if the for-vs-SIMD performance difference would even be noticeable here to begin with)

But I tested with the closed \t:
[it includes a] ^ closing parentheses because \frz360 skips further parsing of \t.

The closing parentheses doesn't matter as it will just be ignored in parsing.
Here's an updated version of the commit with a more correct length in _is_animated_tag and fixing a misplaced block_start = NULL; in _is_event_animated. Can you keep the commit-title and message intact? (If you use git am it will do this by default): fix-animation-detection.patch.txt
Can you also add an explaining description to the commit messages of Fix a lost last event and Fix blend render returning empty result? That is: what was the problem, which render-mode and/or settings did it affect and why is this a good fix. This will make things easier when looking back at those commits in the future when the context is already forgotten.

Can you also add an explaining description to the commit messages of Fix a lost last event and Fix blend render returning empty result? […]
They are simple checks.

With the context in mind they are obvious yes. But when I looked at them again today, a week later, I already forgot about the context and first had to look a bit at the surrounding lines to again understand why this particular early return with all empty images fixes “blend render returning empty result”, or in fact why an empty result is a problem. Putting two explanatory sentences in the commit messages is quick and saves time later on if someone looks at it again in the future.
Fix a lost last event is simpler, but when in doubt it is imho better to err on the side of additional explanation rather than potentially being stuck without needed/helpful explanation/context in the future.

Would you ask about them if they were part of the original PR?

If you mean the backported patches, then in the current situation I'm not asking for those to be changed because they've been deployed in jellyfin for 1+ year, so there's some worth in preserving the actual commit history close to as was.
If however new patches were posted for review/merge like this and I were to decide, I would most definitely insist on the commits being amended to achieve well documented and atomic commits first.

FYI, if you are going to do a "rebase and merge" (for linear history), there are commits that are not atomic.

Yeah, already noticed but thanks for pointing this out again. I'll try to remember to use a merge commit. :)

Bad blend result can be reproduced (without a corresponding commit) with this test5-empty.ass.txt and renderMode: 'blend', dropAllAnimations: true, renderAhead: 0.
It might be better to fix in libass, so that it returns NULL (blank frame) and doesn't return empty sub-images.
Emscripten test is broken. :confused:
Maybe rerun?

@dmitrylyzo : I just tested the AoT Karaoke in Firefox-ESR 91 with
    renderMode: 'blend', // also without
    renderAhead: 9, // also 1 and 90

The subtitles only update when I pause and unpause the video. Without renderAhead everything works as expected.

Unlike fast (lossy), render ahead doesn't skip frames. That's why so animation intensive subtitles such as AoT may stuck.
If worker unable to handle it, you see worker busy, requesting to seek in the browser console. In this case renderAhead is irrelevant - there will be no ready frames.
If you pause and wait, you will see some oneshot received (...), render: XX ms - filling the pre-render cache. When you unpause, you should see updates until the pre-render cache is empty.
Screen resolution plays a lot.
At 1920x1080 Toaru Kagaku no Railgun S renderAhead: 90, I see no problem, because render time fits FPS.
oneshot received (start=16.30820666666668, empty=16.31), render: 15 ms
oneshot received (start=16.341540000000016, empty=16.35), render: 7 ms
oneshot received (start=16.37487333333335, empty=16.38), render: 7 ms
oneshot received (start=16.408206666666686, empty=16.41), render: 4 ms

But at 3840x2160 it stuck.
oneshot received (start=13.305811, empty=13.36), render: 157 ms
(4) worker busy, requesting to seek
oneshot received (start=13.461356, empty=13.56), render: 68 ms
(3) worker busy, requesting to seek
oneshot received (start=13.534487, empty=13.56), render: 56 ms

Screen resolution plays a lot.
At 1920x1080 Toaru Kagaku no Railgun S renderAhead: 90, I see no problem

I didn't fullscreen it, so the physical size should have been below 1920×1080.

Unlike fast (lossy), render ahead doesn't skip frames. That's why so animation intensive subtitles such as AoT may stuck.

Lossy renderer behaves similarly for me, just not quite as bad (this hasn't chagned) and the doc for the renderer already mentions that it will drop frames if they aren't ready in time.
Does renderAhead try to fill the whole prerendering cache before it actually shows anything? If so why? Intuitively I'd expect the prerender cache to fill up independently from the current rendering; if the current frame hasn't been prerendered yet, just render it now (and move the prerendering a bit into the future, possibly using the current desynchronisation as an indicator for how far to skip ahead) — this way it shouldn't be worse than plain blend.

Lossy renderer behaves similarly for me, just not quite as bad (this hasn't chagned) and the doc for the renderer already mentions that it will drop frames if they aren't ready in time. Plain blend is sometimes missing a few frames on heavy sections but will catch up again and behaves the best so far.

AFAIK, the problem with the other modes is that they start rendering at a time when the frame should already be displayed.
If we render fast enough, we don't notice the lack of sync, and renderAhead isn't needed.

Does renderAhead try to fill the whole prerendering cache before it actually shows anything? If so why? Intuitively I'd expect the prerender cache to fill up independently from the current rendering;

When the video is ready, oneshotRender starts requesting the worker for timespans (oneshot-render), and the worker responds with a ready frame for the specified timespan (oneshot-result). This process runs in parallel until the renderAhead limit is reached. But if the worker renders too long, the pre-render cache stops filling, and we get stuck.

if the current frame hasn't been prerendered yet, just render it now (and move the prerendering a bit into the future, possibly using the current desynchronisation as an indicator for how far to skip ahead) — this way it shouldn't be worse than plain blend. Is there already something like this planned?

For on-demand rendering we need a second worker - the current one is busy.
@JustAMan was thinking about multithreaded rendering, iirc. But no one has tried to do that since then.

Just to be sure I tried again with controlled physical canvas sizes, and I noticed that the freezing only happens if the console is already opened While loading the page. If the console is opened later or never it renders smoothly. Any thoughts about that?

Just to be sure I tried again with controlled physical canvas sizes, and I noticed that the freezing only happens if the console is already opened While loading the page. If the console is opened later or never it renders smoothly. Any thoughts about that?

I can correlate the smoother behavior with the closed console (no debug printing), but with the moment it is opened :man_shrugging:

Thanks for the explanation, but I'm afraid without reading the code I'm still not sure about why/how the freeze can come to be. Do all frames of the requested timespan or the cache need to be finished before something is displayed?

It requests them one by one.
This function is called each frame, and if there is a ready frame in the cache, it displays it.
https://github.com/libass/JavascriptSubtitlesOctopus/blob/c8e560cc412d360c042ebf5ec36d36aea230a592/src/subtitles-octopus.js#L375-L408
Then, after the response, tryRequestOneshot is called to request the next timespan.
https://github.com/libass/JavascriptSubtitlesOctopus/blob/c8e560cc412d360c042ebf5ec36d36aea230a592/src/subtitles-octopus.js#L303-L346

When it get's delayed how is the prerendering adjusted?

I am not sure here.
There is a requestNextTimestamp property that is used to move on if we get stuck. But there is no adjustment for the delay.

Yeah, just displaying the last already rendered event may be a simpler fallback. Together with moving the prerendering start to the next (or second or third next) frame (instead of the current frame) and maayyybe also lowering the framereate of animations until the cache catches up to currentTime again, this should in theory prevent freezes and not be worse than plain blend. (Eg half the current framerate  — but at least 1fps — each time a non-prepared frame is requested; when prerendering caught up again reset the framerate to it's original value)

Just a note on parsing transitions: the start of a new transition ends the current transition. I'm not sure if that's already handled. For example: \t(0,130,\1c&HFFFFFF&\t(65,565,\fscy0) is two transitions.

why not 23.976? it ends up being a floating point anyways, so there isn't any perf lost, and it's a bit more accurate to the video framerate

why not 23.976? it ends up being a floating point anyways, so there isn't any perf lost, and it's a bit more accurate to the video framerate

Only if it's an NTSC-video. For PAL and SÈCAM (and the in recent years also appearing exact 24fps videos) it will be less in-line with the video framerate. Not that I think a difference of <2fps matters for browser-rendering, where we can't perfectly sync subs and video anyway.

perhaps also give the prerendering chache more structure than a flat map of frames?

Sorry, didn't get about the cache structure. :sweat_smile:
Currently, it is already an ordered array of object with such info:
{
    eventStart: <number>, // start of the frame (show)
    eventFinish: <number>, // end of the frame (hide)
    emptyFinish: <number>, // end of the gap after
    spentTime: <number>, // time spent in total (for debugging)
    blendTime: <number>, // time spent by blending (for debugging)
    viewport: {
        width: <number>, // width of the frame
        height: <number> // height of the frame
    },
    items: <array of images>, // items to display
    animated: <boolean>, // this frame belongs to animated event (and needs to be subdivided)
    size: <number> // size in bytes
}

Currently, it is already an ordered array of objects with such info:

Oups, I mistakenly thought those objects were always single frames, but items is already an array. Nevermind then.

As this adds multiple options and changes&adds a lot of code, it would be great if I could get another opinion from a JSO-maintainer on this. A summary can be found below.
As a note for testing: Opening the browser-console makes rendering much slower due to the debug prints, which may trigger the (documented and planned to be improved) subtitle-freezes with renderAhead.
The bulk of the changes are backported jellyfin-commits, those add three separate things: 1.) renderAhead to prerender 2.) An option to remove all animating tags 3.) Options to bound the size (in pixels) of the rendering canvas (which will then be scaled by the browser to fit the physical size)
To my understanding it is difficult to split these up because the original commits are interspersed and depend on each other and the commit messages are not always accurately describing what it actually contains and they are definitely not always atomic. (but there's some worth in keeping close to the original commits as they are used in the jellyfin-fork for 1+ year). All of this makes them not only hard to split up but also a major pain to review. The backports are followed up by new commits fixing various issues found in testing and glancing over the code. It appears to pass all our samples from the gh-pages branch.
.
Future todos, probably fine for after this has been merged (in no particular order):

Add a renderAhead sample to our example suit (possibly duplicate the AoT one so it's possible to compare renderAhead+blend with lossy)

investigate whether we can make the WASM-blendRenderer the new default renderer; it seems as it should always be faster than the current default and afaik there are no frame-dropping or browser-incompatibility concerns with it unlike with the lossy "fast"-mode or renderAhead

improve renderAhead's behaviour when falling behind the video (the goal is to not be worse than using a plain blend redererer)

check if renderAhead can be made to work with other rendering-modes

tiled blending patches from jellyfin

DataHoarder's font loading improvements

low-prio: simplify and improve the resizing logic for the prerendering cache

after that: maybe consolidate the (remaining) renderAhead related options into one obejct

WASM blend render isn't worth it on high end hardware, it can't benefit from hardware accelerated canvas blending, so while it might offer insane results on low end hardware, it probably wont on high-end hardware like mine, I tested this on subtitles which  had ~600-1k bitmaps per frame, which performed... okay on my threaded render, but almost not working on blend render
a bit off-topic, but with how stuff is currently going in web, streaming should be a high priority, so we should probably investigate how this behaves on adding events, speaking of which, completely aren't functional https://github.com/libass/JavascriptSubtitlesOctopus/issues/77 and if I remember correctly, renderAhead didn't work well if events were added after it thought there are no more events to pre-render

investigate whether we can make the WASM-blendRenderer the new default renderer; it seems as it should always be faster than the current default

WASM blend render isn't worth it on high end hardware, it can't benefit from hardware accelerated canvas blending, so while it might offer insane results on low end hardware, it probably wont on high-end hardware like mine […]

Defaults are supposed to work well everywhere, not just on some special hardware+software combination.

yes, that is what I'm saying, WASM blend won't work for high intensity TS

Maybe I should've been more clear: Defaults should work reasonably well on all devices and engines. WASM-Blending does and so far seems to always be better then the current default. lossy meanwhile has serious problems on many devices and engines.

investigate whether we can make the WASM-blendRenderer the new default renderer; it seems as it should always be faster than the current default

WASM blend render isn't worth it on high end hardware, it can't benefit from hardware accelerated canvas blending, so while it might offer insane results on low end hardware, it probably wont on high-end hardware like mine […]

Defaults are supposed to work well everywhere, not just on some special hardware+software combination.

yes, that is what I'm saying, WASM blend won't work for high intensity TS

Maybe I should've been more clear: Defaults should work reasonably well on all devices and engines. WASM-Blending does and so far seems to always be better then the current default. fast meanwhile has serious problems on many devices and engines dropping frames and in some cases even being slower than the current default.

lossy performs well on 70% of cases, according to 2021's browser market shares [chrome, edge, firefox], from my testing WASM blending had much more mixed results than fast or even normal, I'd have to re-run these tests after my performance improvements vs this new blend, because the current normal or fast performance is a joke

This PR remains too large to review practically in a single shot. Furthermore, some of the later commits are fixes for bugs introduced in previous commits (e.g. 10c35c587d10ed593b647f1e2cd144fe3736cd59 appears to fix a bug introduced in or after 95bff90e2e46ec3bbd3cf0ddd940c9cf85ddfa30), meaning that the repo state is broken between commits, which makes this unsuitable for merge as-is. Never break functionality in one commit and then fix it in another within the same PR; just rebase and fix the broken commit.
Some changes from this are not actually relevant to this branch, and can be broken up into smaller PRs for separate review and merge review; for instance:

All style cleanup (whitespace changes, etc) is separable from actual code changes
Changes to default settings (e.g. frame rate 30->24) are separable from everything else

Furthermore, some of these commits refactor code that was added in previous commits within the same PR. This makes commit-by-commit review effectively useless. For instance, f38fc7e0ddc37e63b337a0f79b3ddd073bacfabf deletes large amounts of code added in 69906fcc30cf28678cc428db2801f23b44cdffc1. Please squash your commits down appropriately.
Meanwhile, many of these commits add both base-level infrastructure and calls into it in the same commit. This makes review more difficult, and also would make future bisects harder. Implement new C++ functionality in one commit, then call it from JS in another, with each split up into as many commits as appropriate (for instance, adding a new class should usually be a commit on its own).

Most of the commits are not mine - I am just porting a feature from another repo and trying to keep the original authorship of the code.
FPS change is added after https://github.com/libass/JavascriptSubtitlesOctopus/pull/111#discussion_r731431294

Most of the commits are not mine - I am just porting a feature from another repo and trying to keep the original authorship of the code.
FPS change was added after #111 (comment)

I think the original commit structure doesn't matter, just what what does, as long as you put justaman as a co-author it should be fine, but from what I talked about with rcombs, a feature/functionality shouldn't be split into more than 1 commit, or more simply the same code can't be changed in more than 1 commit, and unfortunately this won't be merged until those requirements are met ;-;
so if I was you I wouldn't worry that much about the original commit structure, just focus on the functionality

I'm fine with any editing of the commits, I can recede the "code ownership/authorship" to anyone wanting to maintain this if this helps going further.
I did this in the first place "for the greater good", not for the fame or anything.

Most of the commits are not mine - I am just porting a feature from another repo and trying to keep the original authorship of the code.

There's some value in keeping a close mapping to the original history since it was already deployed publicly for over a year; that's why this form wasn't rejected immediately. However, the non-atomicity and interleaving of multiple changes is too detrimental to the maintainability and bisectability in the long term, while in the short term it makes it very hard to properly review (as the long response times show), making bugs slipping in more likely (we already identified & fixed several bugs, possibly there are even more) and hampering a merge.
Thus, it was decided to forgo preservation of the original history and insist on the general requirements for patches also here.
As for the matter of authorship, there's the widely recognised Co-Authored-By:  commit message trailer which can be used to indicate that multiple people worked on a change, with the original or largest contributor for a particular commit being granted the primary authorship. (I just noticed, you already used this in one commit.)
.
I think, now that we're no longer trying to emulated the original jellyfin-history, this can be split into at least four series (in whatever order is most convenient):

renderAhead mode; possibly also including the proposed changes to its behaviour when falling behind ( + a reminder or patch to add it to the sample suite)
the option to bound the canvas size
the option to drop animations
change fps default, make the WASM-blendRenderer the new default mode and other default changes if there were any

Thanks again for your work on this JustAMan and dmitrylyzo!

Just so it isn't buried under all the comments posted since, here are again the future todos previously identified. The first three (as noted above) and the consolidation of renderAhead-options would probably be best integrated into the newly split series. I don't recall otoh what the resize logic improvements were about, if needed they can be another series before the renderAhead one, to avoid introducing option that change shortly after.

Future todos, [...] (in no particular order):

Add a renderAhead sample to our example suit (possibly duplicate the AoT one so it's possible to compare renderAhead+blend with lossy)

investigate whether we can make the WASM-blendRenderer the new default renderer; it seems as it should always be faster than the current default and afaik there are no frame-dropping or browser-incompatibility concerns with it unlike with the lossy "fast"-mode or renderAhead

improve renderAhead's behaviour when falling behind the video (the goal is to not be worse than using a plain blend redererer)

check if renderAhead can be made to work with other rendering-modes

tiled blending patches from jellyfin

DataHoarder's font loading improvements

low-prio: simplify and improve the resizing logic for the prerendering cache

after that: maybe consolidate the (remaining) renderAhead related options into one obejct

After squashing, you lose all messages of intermediate commits and probably info about why is that. just to warn
Bisecting: there is probably not much difference between a single (squashed) commit and a merge commit when using the --first-parent option (git 2.29).
The Co-Authored-By: doesn't specify the exact part of the code that the co-author touched. I need to know who to blame (even if it's me).
The one that exists is because I reviewed the PR.
dropAnimation feature is a bonus to the animation detector (parser).
Since I will port using the sameoriginal, then fixes pattern, I think you won't accept the PR with the canvas bounding feature as well.
https://github.com/libass/JavascriptSubtitlesOctopus/pull/111/commits/f2c1fead9ba3dea16a2e2b613556fe33e266a6f4
https://github.com/libass/JavascriptSubtitlesOctopus/pull/111/commits/35efbb9b557d63247097bb06627bc16e3735cfb4
https://github.com/libass/JavascriptSubtitlesOctopus/pull/111/commits/a602fd7f68602e1c59a04609f4c93463dd3727c1
https://github.com/libass/JavascriptSubtitlesOctopus/pull/111/commits/c2b711a3f4756a5e789d59372e8e15d6329bfeeb
part of https://github.com/libass/JavascriptSubtitlesOctopus/pull/111/commits/40da83b7063202b247651db5fc940b47093ec1c8
I think you need someone with a more flexible mind.

dropAnimation feature is a bonus to the animation detector (parser).

Adding the animation detection and the option first and then later reusing the detection for renderAhead brings the same end result, but as a bonus takes some bulk away from the renderAhead series.

Bisecting: there is probably not much difference between a single (squashed) commit and a merge commit when using the --first-parent option (git 2.29).

There is a significant difference in all but single-commit merges.
With --first-parent the whole merge is effectively a single (enormous) commit, if the commits making up the merged series are not working properly individually, the series itself cannot be bisected.
A proper patch series consists of well-documented atomic commits, meaning:

each commit compiles individually
each commit is, at the time of merge, assumed to be correct by itself
the state of the whole repo makes sense after each individual commit
there are no temporary user-visible behaviour changes within a series
each commit is as concise/small as possible without breaking any other requirements
(among other that means, refactors are split from the actual code changes)
each commit with a non-trivial change explains in its commit message what it does, why and what assumptions and/or considerations went into the specific implementation of the change

This not only makes review easier, it also means the patch-series itself can be bisected and when you get a result, the amount of code needed to analysse for the source of the bug is much smaller and you have plenty of explanation for what it wants to do and why it was done the way it is, helping in understanding the code and making it less likely new bugs are introduced in an attempt to "fix" the bug currently at hand.
Using --first-parent enables none of this.
Sometimes this can mean the commit message is longer than the actual code change like here. For an example of how the commits are supposed to be split up (but all commits are almost trivial) see the renderMode option unification I split out of this series as an example: #118.

The Co-Authored-By: doesn't specify the exact part of the code that the co-author touched.

If relevant this info can be added to the commit message.

I need to know who to blame (even if it's me).

Pointing fingers to blame when a bug is found is not productive. Fixing the bug is.
Properly documented atomic commits make it easy to locate regressions and to understand how they came about.
An amalgamation of potentially non-compiling and individually bugged commits, makes all of identifying, understanding and fixing harder. Willingly making the code harder to maintain just to make finger-pointing easier is not merely unproductive but detrimental.


Version 6.4.8 - (Hotfix 3)

Downloaded DriverAutomationToolSetup.msi file when launching application it thinks I have an old version .

I'm having this same issue

Yes had same issue

I mean the new installer thinks i already have a newer version


Animations on drains

Animation with the drains on the map – like when you scroll to a place on the map you haven’t been and they all “fall” from the top and how any drain you adopt just keeps bouncing forever.

Adopted drains are hard for people to see.
Once “adopted” a drain it disappeared from the screen and people had to manually refresh the webpage to make it appear again.
After refreshing the adopted drains were being displayed as red and not grey.

The drains do not appear on internet explorer.

@CourtneyCromley, can you clarify your initial comments? Do you want to remove the animation that falls from the top and just have them appear in place?
Is the 4 bounces for the adopted drain acceptable?

Yes I think 4 is acceptable. One of the comments we received from people checking out the site was that the drains never stopped bouncing and they would like them to eventually stop. I think 4 would be a fine amount of bounces.

The second comment is a little confusing, but I have some 'refresh' code done that might resolve the issue. I haven't pushed yet because tests need to be written.

Behavior added by this issue
To begin, My Drains list is empty

When the Adopt! button is pressed, the drain name is automatically listed in My Drains with no refresh needed

When the Abandon this drain button is pressed (and then confirmed in the popup), the drain name is automatically removed from My Drains with no refresh needed


openaps device -h:  ImportError: No module named records

pi@raspberrypi3 ~/openaps $ openaps init myopenaps
Initialized empty Git repository in /home/pi/openaps/myopenaps/.git/
[master (root-commit) 89ef0a2] initializing openaps 0.0.0
0 files changed
create mode 100644 openaps.ini
pi@raspberrypi3 ~/openaps $ cd myopenaps/
pi@raspberrypi3 ~/openaps/myopenaps $ ls
openaps.ini
pi@raspberrypi3 ~/openaps/myopenaps $ openaps device -h
Traceback (most recent call last):
File "/usr/local/bin/openaps-device", line 8, in
execfile(file)
File "/home/pi/openaps/bin/openaps-device", line 9, in
from openaps import devices
File "/home/pi/openaps/openaps/devices/init.py", line 3, in
import add, remove, show
File "/home/pi/openaps/openaps/devices/add.py", line 5, in
from openaps import vendors
File "/home/pi/openaps/openaps/vendors/init.py", line 2, in
import dexcom, medtronic
File "/home/pi/openaps/openaps/vendors/medtronic.py", line 8, in
from decocare import stick, session, link, commands, history
File "/usr/local/lib/python2.7/dist-packages/decocare-0.0.8-py2.7.egg/decocare/history.py", line 11, in
from records import *
ImportError: No module named records

Progress!
Just checking: /home/pi/openaps is empty directory then? (Hopefully not this repo?)
This looks like a similar issue as before, should be fixed now (https://github.com/bewest/decoding-carelink/commit/76014ccbfd8c6929be39fa79dc03eb3069cb0b26).

Try getting latest decocare sources.

dexcom_reader was affected by similar issue.

Yep, pulling latest sources and re-running setup.py fixed it, thanks.


Release version 0.8.0

Not doing so blocks three of my projects from using the latest version of ring.
https://github.com/heartsucker/rust-secure-session/pull/6
https://github.com/heartsucker/iron-csrf/pull/13
https://github.com/heartsucker/rust-csrf/issues/9

Published!


log: yield periodically while dumping logs

The logs are processed in the default task.
In cases where there are a lot of logs to be
processed, the idle task may get blocked from
running, resulting in a watchdog bite. Address
this by periodically putting ourselves on the
sleep list.
Signed-off-by: Naveen Kaje<EMAIL_ADDRESS>

Maybe instead of yielding code could feed watchdog.
If we have two tasks that are busy even if they go to sleep periodically it may happen that they will switch between them and idle will never feed watchdog.
But it could be that watchdog is not initialized at all then feeding it could not be the right thing to do, but yielding without watchdog running will just make walk slower.

@nkaje When it yields to another task, will a problem occur if the new task creates a new log entry while the dump is still in progress?


Inline Javascript Code

Hi,
Is there any way to embed inline JS in SimpleMDE, for example something like a block of JS for adsense or other ads?
Thanks

Well Markdown allows embedding HTML into it as part of the spec. As long as you're not blocking HTML content with SimpleMDE, there shouldn't be a problem.

@Guard13007 Thanks for the answer, I figured it out a couple of days ago.

Hello please can you share the solution

@sennyx Look up how to embed JavaScript in HTML. Markdown allows embedding HTML, therefore you can embed JavaScript in Markdown the same way you do with HTML.


GD-529: Cannot call method 'get_meta' on a null value

The used GdUnit4 version
4.3.3 (Pre Release/Master branch)
The used Godot version
v4.2.1.stable.official [b09f793f5]
Operating System
Windows 11
Describe the bug
In the GdUnit Dock sometimes when right clicking on a test to run/debug it won't work and result in an error in the output. If I select something else, and then try again it will work. It doesn't always happen, but I have a suspicion its related to saving the test script

Steps to Reproduce

Open the application to the GdUnit dock.
Right click a test and debug.
Make some changes to the test script and save with ctrl+s.
Right click a test and debug.
Observe the error in the console (sometimes, not always).

Minimal reproduction project
No response

Hi, thanks for reporting, but I'm not able to reproduce.
Can you please describe in more detail:
a) your gdunit4 settings

auto test discovery on or off
test root directory
b) attach a test suite to reproduce
describe what you have change before save

That didn't actually fix it, just changed the error. It seems related to the tree view and selecting it fresh between test runs.
2024_07_04_0.webm

It occurs only in Godot v4.2.1 and seems to be related to https://github.com/godotengine/godot/pull/88917


Extend kubectl describe for networkPolicy

Is this a BUG REPORT or FEATURE REQUEST? (choose one):
/kind feature
What happened:
Since Allow including both podSelector and namespaceSelector in a NetworkPolicyPeer got merged, we need extend kubectl describe to describe policies where a NetworkPolicyPeer has both podSelector and namespaceSelector set.

@danwinship


feat: easter language toggle

Description
type "english" to see the language toggle button.
it's temporary - once the english texts will be approved, we will release it without a secret code.

after thinking about more about the this change...
may be it can be good to split this language toggle change - to spaller subPoints

sub components?


None is an object not a string.

Another strange piece of code. Line 217 of qrefine/restraints.py is
if self.qm_addon != 'None':
I believe it should be
if self.qm_addon is not None:

For me self.qm_addon is a string with the value 'None' by default, not an object. Testing for is not None is thus always true.
print type(self.qm_addon), self.qm_addon
<type 'str'> None

The error you saw might be because qm_addon_method was not specified (I'll add an error message), but let's resolve this issue first.
I assume it is a string because of the multi-choice. Using 'None' is perhaps counter intuitive.
  qm_addon = *None gcp dftd3
    .type = choice(multi=False)
  qm_addon_method = None
    .type = str

It is a special word in python, like null for java. It is therefore leading to confusion. Need to name it something else.

Ah great! Thanks @pafonine, that is the way to go. Will push the patch soon after some testing.

@hokru what is the status ?

Sorry I lost completely track of this. I pushed a fix now.

@hokru Can we close this?

resolved with a2fbf55d685d99b78ad047c513a9d5bdda94d220


Add city & country data to the log

Can I make an enhancement to this package by adding more information to the request log such as city & country by the requester IP address?
I'm going to integrate it with GeoIP.

@Krisnadi this looks like a good idea. From the django docs it does say that GeoIP is deprecated in favor of GeoIP2. Have you started on this or is there a PR?


c++11 compiler error

I am using trying to compile the DumpInfo example for an Arduino Uno.
Windows 10
RFID version 1.3.6
Arduino 1.8.3
Uno
MFRC 522 (generic,, but not attached for sketch compile test)
I tried copying the unzipped master to the libraries and then including a zipped file.
Both attempts to Verify/compile failed with the c++11 error message.

I am using trying to compile the DumpInfo example for an Arduino Uno.

@rogerrnichols please edit your first post and fill in the required informations. thanks.

Thanks for any help.
Windows 10
RFID version 1.3.6
Arduino 1.8.3
Uno
MFRC 522 (generic,, but not attached for sketch compile test)
I tried copying the unzipped master to the libraries and then including a
zipped file.
Both attempts to Verify/compile failed with the c++11 error message.
On Tue, Aug 15, 2017 at 11:36 AM, Rotzbua<EMAIL_ADDRESS>wrote:

@rogerrnichols https://github.com/rogerrnichols please edit your first
post and fill in the required informations. thanks.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/miguelbalboa/rfid/issues/335#issuecomment-322519406,
or mute the thread
https://github.com/notifications/unsubscribe-auth/ANPzwMdUZ43hXNIIAVMWcBWzWlSZlGSbks5sYckWgaJpZM4O3vRy
.

I could not reproduce your error. Please add a screen shot of the error. Thanks.

Arduino: 1.8.3 (Windows 10), Board: "Arduino Uno"
WARNING: Category '' in library EEPROM is not valid. Setting to
'Uncategorized'
WARNING: Category '' in library SPI is not valid. Setting to 'Uncategorized'
WARNING: Category '' in library SoftwareSerial is not valid. Setting to
'Uncategorized'
WARNING: Category '' in library Wire is not valid. Setting to
'Uncategorized'
Warning: platform.txt from core 'Arduino AVR Boards' contains deprecated
recipe.ar.pattern="{compiler.path}{compiler.ar.cmd}" {compiler.ar.flags}
{compiler.ar.extra_flags} "{build.path}/{archive_file}" "{object_file}",
automatically converted to
recipe.ar.pattern="{compiler.path}{compiler.ar.cmd}" {compiler.ar.flags}
{compiler.ar.extra_flags} "{archive_file_path}" "{object_file}". Consider
upgrading this core.
In file included from
C:\Users\Roger\Documents\Arduino\libraries\rfid-master\src/MFRC522.h:78:0,
             from

C:\Users\Roger\Documents\Arduino\libraries\rfid-master\examples\DumpInfo\DumpInfo.ino:37:
C:\Users\Roger\Documents\Arduino\libraries\rfid-master\src/require_cpp11.h:9:2:
error: #error "This library needs at least a C++11 compliant compiler,
maybe compiler argument for C++11 support is missing or if you use Arduino
IDE upgrade to version >=1.6.6"
#error "This library needs at least a C++11 compliant compiler, maybe
compiler argument for C++11 support is missing or if you use Arduino IDE
upgrade to version >=1.6.6"
^
exit status 1
Error compiling for board Arduino Uno.
This report would have more information with
"Show verbose output during compilation"
option enabled in File -> Preferences.
On Thu, Aug 17, 2017 at 7:00 AM, Rotzbua<EMAIL_ADDRESS>wrote:

I could not reproduce your error. Please add a screen shot of the error.
Thanks.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/miguelbalboa/rfid/issues/335#issuecomment-323051160,
or mute the thread
https://github.com/notifications/unsubscribe-auth/ANPzwGPM5IYPuDvuX_NpFMOzBq0T10Jdks5sZCtIgaJpZM4O3vRy
.

Strange, maybe you have an old toolchain installed. "tools" -> "board manager" -> look for update
rfid-master seems you also did not used the library manager. your description says RFID version 1.3.6 which is wrong. just use releases https://github.com/miguelbalboa/rfid/releases


Field error component

Currently form field error cannot be accessed via DOM so it cannot be via cypress
It should be possible to get field error without mouseover

<FieldError data-cy="" error>
    <FormInput />
</FieldError>

<FormInput>
    <FieldError data-cy="" />
</FormInput>


Improvements

Thanks for the work, this is a good step forward.
To make easier integration of new interpretation framework,
Can you detail in docs    :
API to include new Interpretation models.
For API, building you can refer to this framework   :
https://github.com/arita37/mlmodels

Hello, I will add a section in the Contributing part of doc indeed.
In the mean time, to use the common interface, you need to subclasss the BaseExplainer class:
as you can read in that part of the doc:

will need to define some behaviors when implementing your new explainer. In order to do so, override:

the fit method that defines how (if needed) the explainer should be fited
the feature_importance method that extracts the relative importance of each feature on a dataset globally
the explain_local method that extracts the relative impact of each feature on the final decisionfor every sample in a dataset

Please a template with pre-defined class/methods.
Just easier.

On Feb 27, 2020, at 19:15, aredier<EMAIL_ADDRESS>wrote:

Hello, I will add a section in the Contributing part of doc indeed.
In the mean time, to use the common interface, you need to subclasss the BaseExplainer class:
as you can read in that part of the doc:
will need to define some behaviors when implementing your new explainer. In order to do so, override:
the fit method that defines how (if needed) the explainer should be fited
the feature_importance method that extracts the relative importance of each feature on a dataset globally
the explain_local method that extracts the relative impact of each feature on the final decisionfor every sample in a dataset
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub, or unsubscribe.

#117 should solve this (you can find the methods to override there). If you need a usage example, you can check existing interpreters.

closing this since the branch is merged


Wi-Fi: API disconnects from current network if configure to same network fails

overview
#2159 added old connection cleanup to the wifi configure command, which was good. Unfortunately, this connection cleanup causes a problem if you attempt (for whatever reason) to reconnect to a network you're already connected to.
current behavior
If you attempt (and fail) to connect to the Wi-Fi network your robot is already on, after your attempt your robot will not be on any network.
steps to reproduce

Connect to a wifi network
Attempt to connect to the same network with bad credentials
Notice that your robot will no longer be on a network

expected behavior
Failing to configure a new connection to the network you're already on shouldn't cause you to disconnect.

This is actually not the old connections trimming (that only happens after you successfully connect to the new network), but this segment:
https://github.com/Opentrons/opentrons/blob/78b99c3a5f77d9f314ee0198ec88f35fc93ace49/api/src/opentrons/system/nmcli.py#L448-L453
NetworkManager identifies connections by uuid but still tries to make the human readable network names (which default to the ssid, and we duplicate that behavior for simplicity) unique. So we probably need to change the behavior on reconnect to either:

Set a flag if a connection already exists
If the flag is set, use a unique name for the new connection (by appending "-new" or something)
If the new connection succeeds, trim the old connection and then rename the new one back to the correct name

Or:

If the connection already exists
Save the old connection settings
Modify the connection to have the new settings
If the connection fails, revert to the old settings


TensorflowLite for Windows on Arm

I have been working on making Tensorflow Lite available for the Windows on Arm platform. My current patch can be found here https://github.com/everton1984/tensorflow.git on the branch woa_enablement2. I have written a wiki on how to compile that can be accessed here https://linaro.atlassian.net/wiki/spaces/WOAR/pages/29206609925/Tensorflow+Lite. All that is left is getting the XNNPACK patch into mainstream and I am currently discussing it with the maintaners.
I am wondering if there is the possibility and interest in supporting a new platform, like Windows on Arm and if so if that patch is enough or if there is something else that needs to be tackled.

This issue originally reported by @everton1984 has been moved to this dedicated repository for LiteRT to enhance issue tracking and prioritization. To ensure continuity, we have created this new issue on your behalf.
We appreciate your understanding and look forward to your continued involvement.


Feature 2 Implemented the deck progress tracker

Feature #2 :
Tasks achieved:
✅ The progress is stored in the user's Local Storage
✅  Tracks only the mastered words in each deck
✅ Shows the overall progress for each deck on the home page
Output:

@prithvi206 the deployment has failed, kindly check the logs and fix the build


[BUG] set-output call is deprecated

Is there an existing issue for this?

[X] I have searched the existing issues

Does this issue exist in the latest version?

[X] I'm using the latest release

Describe the bug?
Not a huge bug, but definitely will become a bug in the future...
When using changedfiles, a warning appears reporting the set-output is deprecated. More annoying at the moment, but will completely break in 2023.
https://github.blog/changelog/2022-10-11-github-actions-deprecating-save-state-and-set-output-commands/
To Reproduce
Run: tj_actions/changefiles in a GitHub workflow.
What OS are you seeing the problem on?
all
Expected behavior?
No more warning messages since set-output need not be used.
Relevant log output
The `set-output` command is deprecated and will be disabled soon. Please upgrade to using Environment Files. For more information see: https://github.blog/changelog/2022-10-11-github-actions-deprecating-save-state-and-set-output-commands/

Anything else?
No response
Code of Conduct

[X] I agree to follow this project's Code of Conduct

Oops, Apparently I'm running an older version. Closing.


[Issue #31] (Team 4) keyword matcher refactoring

Refactoring the keyword operator based on the design changes

Added @sandeepreddy602 and @rajesh9625 for their review(s).

I had a pass.  High-level comments: (1) What's the difference between KeywordPredicate and DataReaderPredicate? (2) Format your code properly (especially by adding necessary space).  (3) Rename those Lucene-related variables to make the dependency more explicit.
Given the importance of this refactoring, after you take care of my comments, I want @sandeepreddy602 and @rajesh9625 to review the code.

I found out if the query is anything other than lowercase (both in StringField and TextField) the test cases are failing. I've given one example in above comments.
Below test case is also failing:
@Test
public void testSingleWordQueryInTextField() throws Exception {
//Prepare Query
String query = "Tall";
ArrayList attributeList = new ArrayList<>();
attributeList.add(TestConstants.FIRST_NAME_ATTR);
attributeList.add(TestConstants.LAST_NAME_ATTR);
attributeList.add(TestConstants.DESCRIPTION_ATTR);
    //Prepare expected result list
    List<Span> list = new ArrayList<>();
    Span span = new Span("description", 0, 4, "Tall", "Tall");
    list.add(span);
    Attribute[] schemaAttributes = new Attribute[TestConstants.ATTRIBUTES_PEOPLE.length + 1];
    for(int count = 0; count < schemaAttributes.length - 1; count++) {
        schemaAttributes[count] = TestConstants.ATTRIBUTES_PEOPLE[count];
    }
    schemaAttributes[schemaAttributes.length - 1] = SchemaConstants.SPAN_LIST_ATTRIBUTE;

    IField[] fields1 = { new StringField("bruce"), new StringField("john Lee"), new IntegerField(46),
            new DoubleField(5.50), new DateField(new SimpleDateFormat("MM-dd-yyyy").parse("01-14-1970")),
            new TextField("Tall Angry"), new ListField<>(list) };

    IField[] fields2 = { new StringField("christian john wayne"), new StringField("rock bale"),
            new IntegerField(42), new DoubleField(5.99),
            new DateField(new SimpleDateFormat("MM-dd-yyyy").parse("01-13-1974")), new TextField("Tall Fair"),
            new ListField<>(list) };
    ITuple tuple1 = new DataTuple(new Schema(schemaAttributes), fields1);
    ITuple tuple2 = new DataTuple(new Schema(schemaAttributes), fields2);

    List<ITuple> expectedResultList = new ArrayList<>();
    expectedResultList.add(tuple1);
    expectedResultList.add(tuple2);

    //Perform Query
    List<ITuple> resultList = getPeopleQueryResults(query, attributeList);

    //Perform Check
    boolean contains = TestUtils.containsAllResults(expectedResultList, resultList);
    Assert.assertTrue(contains);
    Assert.assertEquals(2, resultList.size());
}

if I give the query as "tall" the test case is passing, if I give "Tall" or "TALL"  or anything else it's failing.

@rajesh9625  I changed the query to  "TaLL" and it is passing. I dont know why it failed for you.

@akshaybetala  in the span, the key value you added is in lowercase, it should be the given query word. In this case "TaLL" is the given query, same with "BRUCE" also.
Could you check it once:
Span span = new Span("description", 0, 4, "tall", "Tall");  - this is what you've given in the test case.
Ideally it should be Span span = new Span("description", 0, 4, "TaLL", "Tall"); and this fails.

Span will not contain "TaLL" because when the query is tokenized, it is converted to small case.

Yea, that's true. But @chenlica suggested the key should be the actual word given by the user and value is the string found in document. This is what we are doing in DictionaryMatcher. Adding @chenlica to this conversation.

@akshaybetala Agreed with @rajesh9625 . Let's be consistent with DictionaryMatcher by keeping the word by the user.

@akshaybetala please let us know after you take care of comments (including those related to formatting). After that, we will do another round of review.

@akshaybetala This PR is blocking team 1.  Can we finish it ASAP?  If really needed, we can let team 1 take over?

@chenlica Professor Pushed the changes according to the comments. Please take a look.
@rajesh9625 In string field lucene does exact matching. For example if field FIRST_NAME is Bruce then query bruce wont return anything if searched through lucene. This is because lucene doesn't tokenise string field. I think we should follow this convention when using scan source operator.

@sandeepreddy602 and @rajesh9625 : can you do one more review?

I left some minor comments mainly about comments and variable naming.
I wish @sandeepreddy602 can have more review pass.  After he approves it, please do the merge.  Thank you.

Looks good to me. I left 2 minor comments.

Thanks, @sandeepreddy602 .
@akshaybetala and @prakul : please take care of the new comments and do the merge!

@sandeepreddy602 @rajesh9625 @inkudo : since you are blocked by this PR, if @prakul and @akshaybetala are busy, please go ahead to make those minor changes and do the merge!


Polish the plugin a bit

Fix the plugin's identifier to be app.warudo.streamdeck to comply with revers-DNS standards
Fix localization file to not use placeholder text
Clean some unnecessary code and files
Update libs, .gitignore
Prevent websocket from trying to connect without Warudo being open
Show okay/alert icon when pressing a key

Thanks for the PR! Do you know if Elgato will accept identifier changes as part of a plugin update?

No idea, I only started digging into this yesterday.

Lemme reach out and ask them!

@Zarithya We submitted the update last month and still waiting for Elgato to approve it :(

Also, they replied that plugin identifiers cannot be changed, so I think that part of the commit needs to be reverted!

Hm I see, I'm guessing submitting as a new addon wouldn't work? I guess the identifier isn't too important, it just bugs me that it's not revers-DNS compliant 😅


i cant find scan types

No test types found.

I don't understand this ticket. Please provide some info a requested in the template, or explain your problem on Slack.


Rename HeritageClauseElement to ExpressionWithTypeArguments + related methods.

Rename HeritageClauseElement to ExpressionWithTypeArguments + related methods.

:+1:


qlog is back to 500k

Merge by June 30, resolves #32172

@sshane we ready to merge this?

https://github.com/commaai/openpilot/pull/33100


[FEATURE] Standardize casing of tool names on homepage

Is this a unique feature?

[X] I have checked "open" AND "closed" issues and this is not a duplicate

Is your feature request related to a problem/unavailable functionality? Please describe.

The inconsistent letter casing of the tool names on the homepage looks a bit messy.
Languages such as 'JS' or 'CSS' should be uppercase and sometimes aren't.
JSON Generator is the only tool which has an uppercase name.
Markdown is written as 'Mark down' in one of the tool names.

Proposed Solution

All tool names should be changed to follow the majority convention of being 'Title Cased'.
Where language names/ acronyms are use their proper capitalization should be adhered to, or should be a proper noun (capitalized). E.g.:

JavaScript / JS
TypeScript / TS
JSX
CSS
SCSS
JSON
API

Outcome of proposed solution

The website will have a uniform naming convention which looks far better and aids the user in scanning for tools as they will have the expected casing.
Improved accessibility accessibility:

As someone with dyslexia I can attest it's easy to miss a tool your looking for if the letters are in a case other than the one you're initially expecting. This is because our brains are not as fluid at recognizing uppercase and lowercase characters such, as G and g, refer to the same value.

Screenshots
Before

After

Do you want to work on this issue?
Yes
If "yes" to above, please explain how you would technically implement this (issue will not be assigned if this is skipped)

This can be fixed by editing the name of entries in tools.json.

I could add documentation of this naming convention in contributing.md file.

I could also add a checkbox to the pull request template along the lines of:

[ ] New tools adhere to the tool naming convention outlined in contributing.md.

Great idea.

@Bashamega Great, would you like me to add that documentation and update the pull request template too?

@Bashamega Great, would you like me to add that documentation and update the pull request template too?

Yes, thank you very much


Maybe add line indicating historical median through whole graph

Similar to the freezing line, it'd be good to have the historical median have a line across the chart.  This can show more easily whether a value is above the historical median.  I say "maybe" because others may think this is too cluttered, etc, but it does let you see (at a glance) what the historical median is.  Perhaps just put the historical median value directly upon the median line of the box plot?  That should be possible...

Here are some screenshots of experiments with these ideas:

This version wasn't too difficult to implement for a specific chart at a specific browser width, but would be much harder to implement across all possible boxplot heights & browser widths

IMO, these all make the charts feel a tad too cluttered. Adding the deltas from the median to the hovertext (#103) conveys similar information in a different way without adding clutter.

Agree, it's clutter / complexity -- thank you for taking the time to play around with it!  Nice!!


Vitest Documentation in Spanish - Documentación de Vitest en español.

Clear and concise description of the problem
There are a big big comunity that speak Spanish, and Spanish is a forgoten language in software comunity
Hay una comunidad muy grande de habla española, y el español es un lenguaje olvidado en la comunidad de software
Suggested solution
Translate Vitest Documentation to Spanish.
Traducir la documentación de Vitest a español.
Alternative
No response
Additional context
No response
Validations

[X] Follow our Code of Conduct
[X] Read the Contributing Guidelines.
[X] Read the docs.
[X] Check that there isn't already an issue that request the same feature to avoid creating a duplicate.

It is there, you only need to expand navbar:

It is there, you only need to expand navbar:

This is Vite docs, not Vitest

upps, sorry, it is vitest

@Byron2016 there is some work here: https://github.com/miguelfdezc/vitest-docs-es I don't know the current state, the docs can be found here: https://es-vitest.netlify.app/

If you want to support Spanish translation, please join the Discord, and let's talk there.


Read muliple DataTables from one sheet

Is it possible to read multiple tables from a single Sheet into a DataSet?

Table1Col1
Table1Col2

Table1Data1
Table1Data2

Table2Col1
Table2Col2

Table2Data1
Table2Data2

This should result as two DataTables

Possible? Yes, but you have to implement it yourself using the reader APIs.


CI: modify release-notes.py to auto-categorize pull requests based on labels

Which problem is this PR solving?

fixes: https://github.com/jaegertracing/jaeger/issues/4799

Description of the changes

The modifications in release-notes.py would now make sure that the final results printed to the console are in a categorized manner which could be just copy-pasted to release-notes.
The categories object could be modified to accommodate more categories along with some meaningful titles like Changes related to CI: changelog:ci

How was this change tested?

Locally, on the jaegertracing/jaeger repo.

Checklist

[x] I have read https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md
[x] I have signed all commits
[x] I have added unit tests for the new functionality
[x] I have run lint and test steps successfully

for jaeger: make lint test
for jaeger-ui: yarn lint and yarn test

lgtm, but one main comment: we already have a template for release notes in the CHANGELOG, so we want to match the output to that template. That means :

Having a predefined order of categories by importance (breaking changes first, then features, etc.)
Printing category headers with #### markdown (and emojis), to be copyable directly into changelog

What happens when we have commits directly to a branch without a PR?

Having a predefined order of categories by importance (breaking changes first, then features, etc.)

Done. Since Python v3.7, insertion order matters in objects, I have kept the order as it was in previous Changelogs.

What happens when we have commits directly to a branch without a PR?

They all would go to the Others category

We can test the changes more practically, by assigning changelog labels to some of the previously merged PRs after the previous release


I give up trying to get this to run.

Any chance you guys can do an example with Blazor?

Document Details
⚠ Do not edit this section. It is required for learn.microsoft.com ➟ GitHub issue linking.

ID: 4cd79cf8-0944-c752-d6b8-972353c9570c
Version Independent ID: ae44714f-9a20-4cbd-fbb2-4e52f7a41831
Content: Tutorial: Add an Azure SQL database connection in Azure Static Web Apps
Content Source: articles/static-web-apps/database-azure-sql.md
Service: static-web-apps
GitHub Login: @craigshoemaker
Microsoft Alias: cshoe

@gregowahoo Thanks again for reaching here! Unfortunately we don’t have a Blazor version of this doc scheduled at this time.
We will now proceed to close this thread. If there are further questions regarding this matter, please let us know.


Error during training

Hello,
I have some problems with the training. I have followed all the steps for the dataset creation. The only difference is that when I created the dataset, many images were missing.
I used the following command.
python -m siclib.train geocalib-pinhole-openpano     --conf geocalib  --mp float16 data.train_batch_size=2

and this is the error message
[12/10/2024 12:05:23 siclib INFO] [E 0 | it 0] loss {up_angle_error 8.838E+01, up_angle_error_weighted 8.845E+01, up_angle_recall@1 5.371E-04, up_angle_recall@3 1.460E-03, up_angle_recall@5 2.495E-03, up_angle_recall@10 5.386E-03, latitude_angle_error 2.282E+01, latitude_angle_error_weighted 2.284E+01, latitude_angle_recall@1 2.848E-02, latitude_angle_recall@3 7.611E-02, latitude_angle_recall@5 1.233E-01, latitude_angle_recall@10 2.493E-01, roll_error 8.856E+01, pitch_error 1.019E+01, gravity_error 8.891E+01, vfov_error 8.574E+01, k1_error 0.000E+00, stop_at 1.000E+01, initial_up_cost 5.882E-01, initial_latitude_cost 5.897E-02, initial_cost 6.471E-01, final_up_cost 1.195E-01, final_latitude_cost 1.133E-03, final_cost 1.207E-01, up-l1-loss 1.706E+00, up_total 1.706E+00, latitude-l1-loss 3.987E-01, latitude_total 3.987E-01, perspective_total 2.105E+00, gravity 1.857E+00, focal 1.096E+01, dist 0.000E+00, param_total 1.281E+01, total 1.492E+01}
siclib.visualization.visualize_batch.make_perspective_figures
siclib.visualization.visualize_batch.make_perspective_figures
siclib.visualization.visualize_batch.make_perspective_figures
(0, 0)
Traceback (most recent call last):
  File "/home/sebastian/anaconda3/envs/geocalib/lib/python3.9/runpy.py", line 197, in _run_module_as_main
    return _run_code(code, main_globals, None,
  File "/home/sebastian/anaconda3/envs/geocalib/lib/python3.9/runpy.py", line 87, in _run_code
    exec(code, run_globals)
  File "/home/sebastian/GeoCalib/siclib/train.py", line 751, in <module>
    main_worker(0, conf, output_dir, args)
  File "/home/sebastian/GeoCalib/siclib/train.py", line 687, in main_worker
    training(rank, conf, output_dir, args)
  File "/home/sebastian/GeoCalib/siclib/train.py", line 575, in training
    results, pr_metrics, figures = do_evaluation(
  File "/home/sebastian/anaconda3/envs/geocalib/lib/python3.9/site-packages/torch/utils/_contextlib.py", line 116, in decorate_context
    return func(*args, **kwargs)
  File "/home/sebastian/GeoCalib/siclib/train.py", line 152, in do_evaluation
    figures.append(locate(plot_fn)(pred, data))
  File "/home/sebastian/GeoCalib/siclib/visualization/visualize_batch.py", line 189, in make_perspective_figures
    figures |= make_camera_figure(pred, data, n_pairs)
  File "/home/sebastian/GeoCalib/siclib/visualization/visualize_batch.py", line 167, in make_camera_figure
    plot_latitudes(latitudes[i], is_radians=False, axes=ax[i, 1:])
  File "/home/sebastian/GeoCalib/siclib/visualization/viz2d.py", line 467, in plot_latitudes
    return plot_heatmaps(
  File "/home/sebastian/GeoCalib/siclib/visualization/viz2d.py", line 283, in plot_heatmaps
    contours = axes[i].contour(
  File "/home/sebastian/anaconda3/envs/geocalib/lib/python3.9/site-packages/matplotlib/__init__.py", line 1476, in inner
    return func(
  File "/home/sebastian/anaconda3/envs/geocalib/lib/python3.9/site-packages/matplotlib/axes/_axes.py", line 6659, in contour
    contours = mcontour.QuadContourSet(self, *args, **kwargs)
  File "/home/sebastian/anaconda3/envs/geocalib/lib/python3.9/site-packages/matplotlib/contour.py", line 813, in __init__
    kwargs = self._process_args(*args, **kwargs)
  File "/home/sebastian/anaconda3/envs/geocalib/lib/python3.9/site-packages/matplotlib/contour.py", line 1474, in _process_args
    x, y, z = self._contour_args(args, kwargs)
  File "/home/sebastian/anaconda3/envs/geocalib/lib/python3.9/site-packages/matplotlib/contour.py", line 1511, in _contour_args
    x, y = self._initialize_x_y(z)
  File "/home/sebastian/anaconda3/envs/geocalib/lib/python3.9/site-packages/matplotlib/contour.py", line 1587, in _initialize_x_y
    raise TypeError(f"Input z must be at least a (2, 2) shaped array, "
TypeError: Input z must be at least a (2, 2) shaped array, but has shape (0, 0)

Thank you for pointing this out. We have not trained using float16, and this might be causing some issues. I’m looking into this further, but in the meantime, a quick fix would be to comment out the visualization step causing the error at this line. This change will not affect the training process.
Please let me know if you encounter any further issues!

Hello @veichta ,
I made the changes you mentioned.
First, I tried without --mp float16, but the problem persists.
What solved this problem was removing this line. After it, I was able to train using either float32 or float16.
I've got this warning:
[12/12/2024 10:49:44 siclib INFO] Setting epochs to 78 to match num_steps.
[12/12/2024 10:49:44 siclib INFO] Starting epoch 0
/home/sebastian/GeoCalib/siclib/train.py:476: FutureWarning: `torch.cuda.amp.autocast(args...)` is deprecated. Please use `torch.amp.autocast('cuda', args...)` instead.
  with autocast(enabled=args.mixed_precision is not None, dtype=mp_dtype):
[12/12/2024 10:49:59 siclib WARNING] NaN detected in gradient clipping. Skipping iteration.

Could you tell me what does it mean?
Also, regarding the batch size, you mentioned in a paper that a batch size of 24 was used and that 2 GPU cards were used for training. Is 24 the batch size per GPU or the total batch size?
Thanks a lot for your help.

[UPDATE]
The warning disappears when I train the model only on fp32. Does it mean the proposed optimizer doesn't work well with fp16? Have you run any experiments using the model (on inference) with fp16?

Thank you for the update!
The warning should not affect the training as it's just a deprecation warning which might break in future pytorch releases.
As for the batch size, it is the total batch size so for 2 gpus, each one will handle 12 sampels.
We did not do any experiments with float16 on training or inference. But any findings when you experiment with it would be very interesting!


Add notices for ignored and unsupported options when deploying a stack

Proposed changes
Add notices to the Compose file options that are ignored or unsupported when deploying a stack in swarm mode.
I was quite surprised when I tried to deploy a stack in swarm mode with Docker 1.13-rc4 and some options were unsupported. As these options won't be added before the Docker 1.13 release (see related issue), we can at least document them to prevent surprises for others.
Unreleased project version
Docker 1.13 (using the new Compose version 3 format for deploying stacks in swarm mode)
Related issues
See https://github.com/docker/docker/issues/29685

@londoncalling @johndmulhausen PTAL, relates to compose.

@ManoMarks @nathanleclaire @shin- can you all review these changes re: Compose v3 and let me know if they are all correct?

@ManoMarks I just checked, and although there is a discussion of docker stack deploy, no there is no flag on build that I can see. I'll merge this and add that. Thanks!

LGTM, merging, then updating for the qualification on build per @ManoMarks  comment above.

@MagnusS , the flag on the build was in there! Thanks!


🐛 Fixed ability to submit invite form with a missing role

refs https://github.com/TryGhost/Ghost/issues/10071

moved roles fetch request into an ember-concurrency task
use the task's derived state to disable the submit button whilst fetching
added role presence check to the invite user validator and updated template to display the error

Coverage decreased (-0.03%) to 71.34% when pulling 452aad74076ba125147b9c3279c173cb9190b1cf on kevinansfield:fix-missing-invite-role into 02b750e64e9dd8c45b650961a5f6c8430417ee24 on TryGhost:master.


RE: Tankbusters always sound

Summary
This is regarding https://github.com/quisquous/cactbot/pull/3432, and the change to remove the "CaresAbout" conditionals.  This appears to be somewhat inconsistent, as some conditions (ex. Pillory in Holminster Switch) are still using an ad-hoc version.
However, the point is more around the "well, if users don't like it, they can disable it" conclusion.  The issue, for me, is that I play multiple roles.  I don't care about tankbusters as a DPS, but I very much do care about them when I'm tanking or healing.  I'd like to propose either:

A global "don't show tank alerts as DPS" option (and maybe also one for healer and tank, but eh).  This would apply to tankbusters for sure, and maybe also tank cleaves and other tank-centered effects that DPS generally don't have to muck with.

OR:

A new enhancement to the config in the UI that lets the user specify that the alerts should only fire for X roles.  This would be be a series of checkboxes (probably a horizontal row, but UI design isn't my forte) listing the 5 roles in FFXIV (tank, healer, melee, ranged, caster).  Default checked state could be all on, or defined by the trigger, or only tanks and healers, whatever seems to make the most sense.  But that would let users directly control whether each trigger showed for any given role, without having to manually go in and change them every time they decide to swap to tanking or healing for a change.

Edit: also, the intent with this one is that it'd apply globally to all alerts, providing the customization regardless of the alert type (ex. if DPS or tanks don't care about general raid AoE hits that are there mostly just to give healers something to do).

Personally, I prefer the second option, as it affords the greater amount of control and flexibility (ie. can set a tankbuster on one fight to show but not on another, depending on relevance to your role), but I'm not sure on the relative effort between the two, or which y'all feel would make more sense from a UX perspective.

As with nearly everything, it's a tradeoff.

It's a fair point.  And mine apologies if I made it sound like this is merely stubbornness on your part.  As you say, I think this ultimately comes down to a judgment on how many users are likely to engage with the feature, versus how many are not.
The current options for customizing notifications are fairly basic, being confined to a handful of options as far as alert type, and then an option to set the actual text displayed.  All other customization requires at least some knowledge of and comfort with editing code files.  While the actual implementation of those custom triggers is likely something you (and I) find to be quite easy, from a mechanical perspective, I know many who would be overwhelmed by the mere prospect of trying to put together a custom trigger, regardless of how many help comments and examples were given, and regardless of how much of it was, tbh, essentially copy-paste with some tweaking.  I think software engineers like you and I sometimes forget how daunting looking at code can be for those not familiar with coding as a concept.

That's why "maintain this feature across all triggers" has a cost.

But that was really my point.  Handling this on a trigger-by-trigger basis is a huge amount of work, both now and for every single feature going forward.  If, on the other hand, this were implemented in a single common function that all of the triggers leverage (or even a handful of them), like the same config set you store the existing options for displaying text, sound, and/or TTS, that the solution would be a set-it-and-forget-it without needed maintenance except where code that affects it actually changes.  The default for triggers without a configured set of options could be all-roles-selected, and then maybe the triggers could override it if the trigger needs to be specific (but you'd only need to define it for triggers that are specific, and only if you want to go that direction; the existing role conditions are probably fine if you don't).
Now, unfortunately, I don't know Typescript (or Javascript, for that matter, my primary language is Go), and looking over the code for this plugin is in fact the first time I've encountered Typescript code, else I would have simply PR'd the changes by now.  I have a vague idea of where the code for it might live and how it might work, but my knowledge of TS simply isn't up to the task ('yet', potentially).

The changes and the workaround are both roughly the same set of work.

Only if you presume the only possible solution on the plugin side is to edit every single trigger.  And that's not what I'm proposing.
I'm proposing a new option created (I'd assume this would be in a single place, as I assume each trigger simply uses a template to generate its options section in the UI), and then the selection for those would be leveraged in a single or possible a handful of places, depending on how the triggers are actually handled (tbh, it's a bit difficult following the program flow, not knowing the particulars of TS syntax), where in some common function each of the triggers ultimately passes through before being displayed, it would check that optionset and simply return without doing anything if the player's role did not match the optionset.  If the ability for triggers to set a default set of roles (thus obviating the need for role-based trigger conditions, though that's an optional enhancement, imo), that would be handled when generating the UI options, in much the same way that triggers can define the default alert text, duration, etc (which can then be overridden by the user).
Can you not see how there's a world of difference in the effort required between copying 70+ triggers, probably per expansion, versus implementing a single approach in a common function that automatically applies to all triggers merely by virtue of their existing templating?

Coming back to old issues.  I'm sorry for the disagreement here, but if this is something that you want I think your best bet is to fork cactbot and implement it yourself.  I haven't heard anybody else ask for this or complain about the tankbusters in endwalker.


Having problem using ODAS Studio

Hey there,
I downloaded ODAS Studio for windows. When I run ODAS Studio and set ODAS Control part, consisting of ODAS Core and ODAS Configure, then when I press "Launch ODAS" button, an error occurs. I have attached a photo that shows the error. Please help me with this issue.
Thank you .

Hi, this message means that ODAS Studio is not able to start the ODAS process.
I would suggest you try to launch it in command line rather than trough the Studio to see if you can get more information.
Also, you seem to be running Windows. Are you trying to run ODAS on windows? As far as I know, the ODAS core can only be run on Linux.

That's right.
Could you tell me please how to run that in command line, I mean, what exactly should I write there?
I'm running on Windows. But I have already launched it on Linux, I've got the ".bin" output there, then I added ".bin" file to my ODAS Core in Windows. It isn't working. Any suggestions on windows?
Thanks.

Per the ODAS Wiki:
./odaslive -c myConfigFile.cfg
You can't run the ODAS Core in Windows. It uses libraries such as Alsa which are only on Linux.
Furthermore, you cannot build a binary file for Linux then copy it to Windows and run it. It must be built for the correct platform.


Fix for SourceComments including stdin instead of proper file path

Resolves https://github.com/dlmanning/gulp-sass/issues/234

sorry for the wait, could you add a test that was previously failing for that to the testsuite?

Yes, please add a test for this.
I'm also concerned that passing the file to Node Sass may result in unintended consequences of the raw file being compiled instead of the contents of the file stream. We already have a place in our codebase to transform stdin in to the main file's path (for sourcemaps); please refer to that,

With the fix in #314, this is now failing. Can you please update?

@xzyfer are you still interested in this? I rebased it but unsure if you want to land it: https://github.com/dlmanning/gulp-sass/compare/master...XhmikosR:pr/286


imu-frames-timestamp-sync.py  got type error

use oakd-w-pro. And try python gen2-syncing/imu-frames-timestamp-sync.py
it return following message:
FPS<PHONE_NUMBER>258327
Traceback (most recent call last):
  File "gen2-syncing/imu-frames-timestamp-sync.py", line 185, in <module>
    new_msg(msg, name, ts)
  File "gen2-syncing/imu-frames-timestamp-sync.py", line 153, in new_msg
    print(f"[Seq {rgb.getSequenceNum()}] Mid of RGB exposure ts: {td2ms(rgb_ts)}ms, RGB ts: {td2ms(rgb.getTimestampDevice())}ms, RGB exposure time: {td2ms(rgb.getExposureTime(), exposure=True)}ms")

I used depthai-<IP_ADDRESS>

Hi @ryanESX ,
Could you copy the full error message?
Thanks, Erik


WIP Build the general documentation with mkdocs

This uses mkdocs instead of docfx to build the website. Advantages:

mkdocs, and particularly the material theme this PR uses, has great support and is a very active project with many options and plugins.
The code samples now have a scrollbar, so all code can be seen without having to use wrap around.
The code samples now have an option to copy the code when hovering over it.
For the quickstarts we can now use a tabbed content block, making it more compact, and the start page becomes the single page for everyone to get started. My plan is to add more documentation after the quickstarts (on the same page) taking the users along with some more samples (the basics of navigation, adding points, adding widgets, etc).

The mkdocs site is currently deployed here: https://mapsui.com/documentation2/
At the moment I do not have plans to port the api documentation to mkdocs. There are quite a number of plugins for api documentation in mkdocs (look here), so I guess it is possible for Mapsui as well, but because we have a C# project it might be problematic. This means we have to deploy mkdocs and docfx side by side and integrate it somehow or just link from one to the other.

This is half done but I will merge it because (as far as I know) it does not break the existing website.


Update install file to copy json file into bin

Im guessing that you will have to copy the json file also into bin. Do update install.sh appropriately

Traceback (most recent call last):
  File "/bin/never_kill_your_time", line 37, in <module>
    json_data=open("routine.json").read()
IOError: [Errno 2] No such file or directory: 'routine.json'

I get this error when I try to run never_kill_your_time from a desktop (Note: I have already installed script on bin). I don't have an idea why is it showing this type of behavior.

This commit solves it. thanks for pointing it out!

Works yahoo :)


Remove teams v1 server

Nothing is using the IAM v1 teams server any more so it is safe to delete it:
components/teams-service/server/v1/server.go
api/interservice/teams/v1/teams.proto

Also should review the tests to see if any should be added to the v2 suite.
components/teams-service/server/v1/server_test.go

will be addressed in #3273


PROPOSAL: Make search by file name case insensitive

Greetings and thanks for the plugin, it's quite usefull!
It would be nice to have the ability to search by file name without worrying with lower/upper case.

Actually it was case-insensitive from the beginning 😅
Anyway, added an additional option for this in the latest release:
https://github.com/Amonith/mattermost-file-list/releases/tag/v0.9.0


hrRoot insert custom headers

Hey,
I was trying to use this great library for a personal project.
The reference API can emit hal or siren payload based on the Accept header.
From the examples that I saw here, looks like
start = hrRoot('http://reference.com')

is the way to start working with it.
But how can I set a custom header before issuing the request itself?

Hello,
thank you very much for the prompt reply and fix.
Would be possibile it on bower as well?
Keep going the great work!

@XVincentX I should have time to push a new release of angular-hy-res to Bower tonight.

@XVincentX  The Accept header sent by angular-hy-res in sample app happens because the links that initiate those requests include a type value. See https://github.com/kevinswiber/siren#type for Siren or http://tools.ietf.org/html/draft-kelly-json-hal-06#section-5.3 for HAL. Ultimately, this is part of the RFC5899 Web Linking standard.
For modifying the Accept header sent when fetching the root of the API, you have two options using the just-published v0.0.13:

I updated hrRoot to actually return a WebLink instance itself, which can then be followed. If you set the type value on the WebLink instance, when you then invoke follow(), it should work as expected, e.g.:
 var rootLink = hrRoot('http://reference.com');
 rootLink.type = 'application/hal+json';
 var root = rootLink.follow();

You can use the HTTP options that are passed to the underlying $http AngularJS service, like:
 var root = hrRoot('http://reference.com').follow({ headers: { Accept: 'application/hal+json' } });

One option I've considered is creating some "default options" that would allow specifying something like: hrRoot('http://reference.com', { automaticAcceptHandling: true }) that would populate the Accept header sent based on the registered extensions. Thoughts?

Hey,
thank you for the prompt update, I am going to try that right now.
The auto detect option would be very interesting.
The only question that comes into my mind actually is: what if I've registered both extensions (siren and hal, for example). Which Accept-Header would you send? Throwing an exception? Sending the first one given the registration order?

@XVincentX I think you would send an Accept header with both, in the order they are registered. If we wanted to offer a bit more flexibility there, it could add a q parameter  based on a quality property on the extension...

I agree for both types in the Accept header.
IMHO I'd skip the quality parameter.
Thank you

I guess we can close this.


After applying a policy, Galley crashes

Working with Istio 1.1.7
Studying how to use JWTs, I applied the following YAML file
apiVersion: "authentication.istio.io/v1alpha1"
kind: Policy
metadata:
name: front-end-policy
spec:
targets:

name: front-end
origins:
jwt:
issuer<EMAIL_ADDRESS>jwksUri: "https://raw.githubusercontent.com/istio/istio/release-1.1/security/tools/jwt/samples/jwks.json"
principalBinding: USE_ORIGIN

I get this erro
Error from server (InternalError): error when creating "policy.yaml": Internal error occurred: failed calling admission webhook "pilot.validation.istio.io": Post https://istio-galley.istio-system.svc:7443/admitpilot?timeout=30s: stream error: stream ID 1; INTERNAL_ERROR
The Galley logs show
2019-06-13T11:22:49.757037Z     info    http2: panic serving <IP_ADDRESS>:35736: runtime error: invalid memory address or nil pointer dereference
goroutine 1442 [running]:
net/http.(*http2serverConn).runHandler.func1(0xc4203daae0, 0xc420be9faf, 0xc42050c1c0)
/usr/local/go/src/net/http/h2_bundle.go:5753 +0x190
panic(0x1ca6dc0, 0x2fd1910)
/usr/local/go/src/runtime/panic.go:502 +0x229
istio.io/istio/pilot/pkg/model.ValidateAuthenticationPolicy(0xc420c818b0, 0x10, 0xc420c818a7, 0x7, 0x21be3c0, 0xc420474100, 0xe, 0x1fab594)
/workspace/go/src/istio.io/istio/pilot/pkg/model/validation.go:1457 +0x58d
istio.io/istio/galley/pkg/crd/validation.(*Webhook).admitPilot(0xc420692b40, 0xc420e96000, 0xc4201bef00)
/workspace/go/src/istio.io/istio/galley/pkg/crd/validation/webhook.go:472 +0x509
istio.io/istio/galley/pkg/crd/validation.(*Webhook).(istio.io/istio/galley/pkg/crd/validation.admitPilot)-fm(0xc420e96000, 0xc420d9b000)
/workspace/go/src/istio.io/istio/galley/pkg/crd/validation/webhook.go:435 +0x34
istio.io/istio/galley/pkg/crd/validation.serve(0x21cb6c0, 0xc4203daae0, 0xc420d4a200, 0xc420be9cc0)
/workspace/go/src/istio.io/istio/galley/pkg/crd/validation/webhook.go:407 +0x4ee
istio.io/istio/galley/pkg/crd/validation.(*Webhook).serveAdmitPilot(0xc420692b40, 0x21cb6c0, 0xc4203daae0, 0xc420d4a200)
/workspace/go/src/istio.io/istio/galley/pkg/crd/validation/webhook.go:435 +0x5f
istio.io/istio/galley/pkg/crd/validation.(*Webhook).(istio.io/istio/galley/pkg/crd/validation.serveAdmitPilot)-fm(0x21cb6c0, 0xc4203daae0, 0xc420d4a200)
/workspace/go/src/istio.io/istio/galley/pkg/crd/validation/webhook.go:287 +0x48
net/http.HandlerFunc.ServeHTTP(0xc4200b7c10, 0x21cb6c0, 0xc4203daae0, 0xc420d4a200)
/usr/local/go/src/net/http/server.go:1947 +0x44
net/http.(*ServeMux).ServeHTTP(0xc4206919b0, 0x21cb6c0, 0xc4203daae0, 0xc420d4a200)
/usr/local/go/src/net/http/server.go:2340 +0x130
net/http.serverHandler.ServeHTTP(0xc42061fe10, 0x21cb6c0, 0xc4203daae0, 0xc420d4a200)
/usr/local/go/src/net/http/server.go:2697 +0xbc
net/http.initNPNRequest.ServeHTTP(0xc4204bf500, 0xc42061fe10, 0x21cb6c0, 0xc4203daae0, 0xc420d4a200)
/usr/local/go/src/net/http/server.go:3263 +0x9a
net/http.(Handler).ServeHTTP-fm(0x21cb6c0, 0xc4203daae0, 0xc420d4a200)
/usr/local/go/src/net/http/h2_bundle.go:5475 +0x4d
net/http.(*http2serverConn).runHandler(0xc42050c1c0, 0xc4203daae0, 0xc420d4a200, 0xc420c4d600)
/usr/local/go/src/net/http/h2_bundle.go:5760 +0x89
created by net/http.(*http2serverConn).processHeaders
/usr/local/go/src/net/http/h2_bundle.go:5494 +0x46b
Any ideas, please?

If I remove all the lines about Jwt, there is no error (but meaningless in my case)

kind: Policy
metadata:
  name: front-end-policy
spec:
  targets:
  - name: front-end
  origins:
  - jwt:
      issuer<EMAIL_ADDRESS>      jwksUri: "https://raw.githubusercontent.com/istio/istio/release-1.1/security/tools/jwt/samples/jwks.json"
  principalBinding: USE_ORIGIN``` this config works.

@wattli Is this something that you can take a look? The crash is coming from the validation code within Pilot.

istio.io/istio/pilot/pkg/model.ValidateAuthenticationPolicy(0xc420c818b0, 0x10, 0xc420c818a7, 0x7, 0x21be3c0, 0xc420474100, 0xe, 0x1fab594)```

@daleangus Could you share the steps to reproduce this error? How did you install Istio?, etc.

I installed Istio 1.1.8 with mTLS enabled. I have grafana and kiali enabled. I installed my application. Everything is working until I apply this policy with jwt.

@wattli @diemtvu @lei-tang
Folks, can one of you route this appropriately and get it fixed?

This should already be fixed by https://github.com/istio/istio/pull/14910 in 1.2.
@daleangus you missed a few whitespaces before the issuer and jwksUri field, the correct yaml should be like the following:
apiVersion: "authentication.istio.io/v1alpha1"
kind: Policy
metadata:
  name: front-end-policy
spec:
  targets:
  - name: front-end
  origins:
  - jwt:
      issuer<EMAIL_ADDRESS>      jwksUri: "https://raw.githubusercontent.com/istio/istio/release-1.1/security/tools/jwt/samples/jwks.json"
  principalBinding: USE_ORIGIN

I'm closing this as it's already fixed. Please feel free to reopen if you have more questions. Thanks!